From d303557fc601c8b617d34dded401bf85038500ce Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sun, 15 Nov 2020 15:28:11 +0300 Subject: Implements Channel Converter Adds a converter that can decipher more forms of channel mentions, to lay foundation for voice channel muting. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/converters.py | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/bot/converters.py b/bot/converters.py index 2e118d476..b9db37fce 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -536,6 +536,46 @@ class FetchedUser(UserConverter): raise BadArgument(f"User `{arg}` does not exist") +class AnyChannelConverter(UserConverter): + """ + Converts to a `discord.Channel` or, raises an error. + + Unlike the default Channel Converter, this converter can handle channels given + in string, id, or mention formatting for both `TextChannel`s and `VoiceChannel`s. + Always returns 1 or fewer channels, errors if more than one match exists. + + It is able to handle the following formats (caveats noted below:) + 1. Convert from ID - Example: 267631170882240512 + 2. Convert from Explicit Mention - Example: #welcome + 3. Convert from ID Mention - Example: <#267631170882240512> + 4. Convert from Unmentioned Name: - Example: welcome + + All the previous conversions are valid for both text and voice channels, but explicit + raw names (#4) do not work for non-unique channels, instead opting for an error. + Explicit mentions (#2) do not work for non-unique voice channels either. + """ + + async def convert(self, ctx: Context, arg: str) -> t.Union[discord.TextChannel, discord.VoiceChannel]: + """Convert the `arg` to a `TextChannel` or `VoiceChannel`.""" + stripped = arg.strip().lstrip("<").lstrip("#").rstrip(">") + + # Filter channels by name and ID + channels = [channel for channel in ctx.guild.channels if stripped in (channel.name, str(channel.id))] + + if len(channels) == 0: + # Couldn't find a matching channel + log.debug(f"Could not convert `{arg}` to channel, no matches found.") + raise BadArgument("The provided argument returned no matches.") + + elif len(channels) > 1: + # Couldn't discern the desired channel + log.debug(f"Could not convert `{arg}` to channel, {len(channels)} matches found.") + raise BadArgument(f"The provided argument returned too many matches ({len(channels)}).") + + else: + return channels[0] + + def _snowflake_from_regex(pattern: t.Pattern, arg: str) -> int: """ Extract the snowflake from `arg` using a regex `pattern` and return it as an int. -- cgit v1.2.3 From 13866a11f764dec27bc5a600781caa17986e9957 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 16 Nov 2020 01:48:38 +0300 Subject: Adds VoiceChannel Mute Adds an optional channel parameter to silence and unsilence commands, and adds ability to silence voice channels. TODO: New Tests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/constants.py | 2 + bot/exts/moderation/silence.py | 165 ++++++++++++++++++++++-------- tests/bot/exts/moderation/test_silence.py | 12 +-- 3 files changed, 132 insertions(+), 47 deletions(-) diff --git a/bot/constants.py b/bot/constants.py index 731f06fed..e41be5927 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -395,6 +395,8 @@ class Channels(metaclass=YAMLGetter): change_log: int code_help_voice: int code_help_voice_2: int + admins_voice: int + staff_voice: int cooldown: int defcon: int dev_contrib: int diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index e6712b3b6..266669eed 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -1,18 +1,19 @@ import json import logging +import typing from contextlib import suppress from datetime import datetime, timedelta, timezone from operator import attrgetter from typing import Optional from async_rediscache import RedisCache -from discord import TextChannel +from discord import TextChannel, VoiceChannel from discord.ext import commands, tasks from discord.ext.commands import Context from bot.bot import Bot from bot.constants import Channels, Emojis, Guild, MODERATION_ROLES, Roles -from bot.converters import HushDurationConverter +from bot.converters import HushDurationConverter, AnyChannelConverter from bot.utils.lock import LockedResourceError, lock_arg from bot.utils.scheduling import Scheduler @@ -41,7 +42,7 @@ class SilenceNotifier(tasks.Loop): self._silenced_channels = {} self._alert_channel = alert_channel - def add_channel(self, channel: TextChannel) -> None: + def add_channel(self, channel: typing.Union[TextChannel, VoiceChannel]) -> None: """Add channel to `_silenced_channels` and start loop if not launched.""" if not self._silenced_channels: self.start() @@ -93,14 +94,54 @@ class Silence(commands.Cog): await self.bot.wait_until_guild_available() guild = self.bot.get_guild(Guild.id) - self._verified_role = guild.get_role(Roles.verified) + self._verified_msg_role = guild.get_role(Roles.verified) + self._verified_voice_role = guild.get_role(Roles.voice_verified) self._mod_alerts_channel = self.bot.get_channel(Channels.mod_alerts) self.notifier = SilenceNotifier(self.bot.get_channel(Channels.mod_log)) await self._reschedule() + async def send_message(self, message: str, source_channel: TextChannel, + target_channel: typing.Union[TextChannel, VoiceChannel], + alert_target: bool = False, duration: HushDurationConverter = 0) -> None: + """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`""" + if isinstance(source_channel, TextChannel): + await source_channel.send( + message.replace("current", target_channel.mention if source_channel != target_channel else "current") + .replace("{duration}", str(duration)) + ) + + voice_chat = None + if isinstance(target_channel, VoiceChannel): + # Send to relevant channel + # TODO: Figure out a non-hardcoded way of doing this + if "offtopic" in target_channel.name.lower(): + voice_chat = self.bot.get_channel(Channels.voice_chat) + elif "code-help" in target_channel.name.lower(): + if "1" in target_channel.name.lower(): + voice_chat = self.bot.get_channel(Channels.code_help_voice) + else: + voice_chat = self.bot.get_channel(Channels.code_help_voice_2) + elif "admin" in target_channel.name.lower(): + voice_chat = self.bot.get_channel(Channels.admins_voice) + elif "staff" in target_channel.name.lower(): + voice_chat = self.bot.get_channel(Channels.staff_voice) + + if alert_target and source_channel != target_channel: + if isinstance(target_channel, VoiceChannel): + if voice_chat is None or voice_chat == source_channel: + return + + await voice_chat.send( + message.replace("{duration}", str(duration)).replace("current", voice_chat.mention) + ) + + else: + await target_channel.send(message.replace("{duration}", str(duration))) + @commands.command(aliases=("hush",)) @lock_arg(LOCK_NAMESPACE, "ctx", attrgetter("channel"), raise_error=True) - async def silence(self, ctx: Context, duration: HushDurationConverter = 10) -> None: + async def silence(self, ctx: Context, duration: HushDurationConverter = 10, + channel: AnyChannelConverter = None) -> None: """ Silence the current channel for `duration` minutes or `forever`. @@ -108,72 +149,100 @@ class Silence(commands.Cog): Indefinitely silenced channels get added to a notifier which posts notices every 15 minutes from the start. """ await self._init_task - - channel_info = f"#{ctx.channel} ({ctx.channel.id})" + if channel is None: + channel = ctx.channel + channel_info = f"#{channel} ({channel.id})" log.debug(f"{ctx.author} is silencing channel {channel_info}.") - if not await self._set_silence_overwrites(ctx.channel): + if not await self._set_silence_overwrites(channel): log.info(f"Tried to silence channel {channel_info} but the channel was already silenced.") - await ctx.send(MSG_SILENCE_FAIL) + await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel) return - await self._schedule_unsilence(ctx, duration) + await self._schedule_unsilence(ctx, channel, duration) if duration is None: - self.notifier.add_channel(ctx.channel) + self.notifier.add_channel(channel) log.info(f"Silenced {channel_info} indefinitely.") - await ctx.send(MSG_SILENCE_PERMANENT) + await self.send_message(MSG_SILENCE_PERMANENT, ctx.channel, channel, True) + else: log.info(f"Silenced {channel_info} for {duration} minute(s).") - await ctx.send(MSG_SILENCE_SUCCESS.format(duration=duration)) + await self.send_message(MSG_SILENCE_SUCCESS, ctx.channel, channel, True, duration) @commands.command(aliases=("unhush",)) - async def unsilence(self, ctx: Context) -> None: + async def unsilence(self, ctx: Context, channel: AnyChannelConverter = None) -> None: """ - Unsilence the current channel. + Unsilence the given channel if given, else the current one. If the channel was silenced indefinitely, notifications for the channel will stop. """ await self._init_task - log.debug(f"Unsilencing channel #{ctx.channel} from {ctx.author}'s command.") - await self._unsilence_wrapper(ctx.channel) + if channel is None: + channel = ctx.channel + log.debug(f"Unsilencing channel #{channel} from {ctx.author}'s command.") + await self._unsilence_wrapper(channel, ctx) @lock_arg(LOCK_NAMESPACE, "channel", raise_error=True) - async def _unsilence_wrapper(self, channel: TextChannel) -> None: + async def _unsilence_wrapper(self, channel: typing.Union[TextChannel, VoiceChannel], + ctx: typing.Optional[Context] = None) -> None: """Unsilence `channel` and send a success/failure message.""" + msg_channel = channel + if ctx is not None: + msg_channel = ctx.channel + if not await self._unsilence(channel): - overwrite = channel.overwrites_for(self._verified_role) - if overwrite.send_messages is False or overwrite.add_reactions is False: - await channel.send(MSG_UNSILENCE_MANUAL) + if isinstance(channel, VoiceChannel): + overwrite = channel.overwrites_for(self._verified_voice_role) + manual = overwrite.speak is False + else: + overwrite = channel.overwrites_for(self._verified_msg_role) + manual = overwrite.send_messages is False or overwrite.add_reactions is False + + # Send fail message to muted channel or voice chat channel, and invocation channel + if manual: + await self.send_message(MSG_UNSILENCE_MANUAL, msg_channel, channel) else: - await channel.send(MSG_UNSILENCE_FAIL) + await self.send_message(MSG_UNSILENCE_FAIL, msg_channel, channel) + else: - await channel.send(MSG_UNSILENCE_SUCCESS) + # Send success message to muted channel or voice chat channel, and invocation channel + await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, True) - async def _set_silence_overwrites(self, channel: TextChannel) -> bool: + async def _set_silence_overwrites(self, channel: typing.Union[TextChannel, VoiceChannel]) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" - overwrite = channel.overwrites_for(self._verified_role) - prev_overwrites = dict(send_messages=overwrite.send_messages, add_reactions=overwrite.add_reactions) + if isinstance(channel, TextChannel): + overwrite = channel.overwrites_for(self._verified_msg_role) + prev_overwrites = dict(send_messages=overwrite.send_messages, add_reactions=overwrite.add_reactions) + else: + overwrite = channel.overwrites_for(self._verified_voice_role) + prev_overwrites = dict(speak=overwrite.speak) if channel.id in self.scheduler or all(val is False for val in prev_overwrites.values()): return False - overwrite.update(send_messages=False, add_reactions=False) - await channel.set_permissions(self._verified_role, overwrite=overwrite) + if isinstance(channel, TextChannel): + overwrite.update(send_messages=False, add_reactions=False) + await channel.set_permissions(self._verified_msg_role, overwrite=overwrite) + else: + overwrite.update(speak=False) + await channel.set_permissions(self._verified_voice_role, overwrite=overwrite) + await self.previous_overwrites.set(channel.id, json.dumps(prev_overwrites)) return True - async def _schedule_unsilence(self, ctx: Context, duration: Optional[int]) -> None: + async def _schedule_unsilence(self, ctx: Context, channel: typing.Union[TextChannel, VoiceChannel], + duration: Optional[int]) -> None: """Schedule `ctx.channel` to be unsilenced if `duration` is not None.""" if duration is None: - await self.unsilence_timestamps.set(ctx.channel.id, -1) + await self.unsilence_timestamps.set(channel.id, -1) else: - self.scheduler.schedule_later(duration * 60, ctx.channel.id, ctx.invoke(self.unsilence)) + self.scheduler.schedule_later(duration * 60, channel.id, ctx.invoke(self.unsilence, channel=channel)) unsilence_time = datetime.now(tz=timezone.utc) + timedelta(minutes=duration) - await self.unsilence_timestamps.set(ctx.channel.id, unsilence_time.timestamp()) + await self.unsilence_timestamps.set(channel.id, unsilence_time.timestamp()) - async def _unsilence(self, channel: TextChannel) -> bool: + async def _unsilence(self, channel: typing.Union[TextChannel, VoiceChannel]) -> bool: """ Unsilence `channel`. @@ -188,14 +257,21 @@ class Silence(commands.Cog): log.info(f"Tried to unsilence channel #{channel} ({channel.id}) but the channel was not silenced.") return False - overwrite = channel.overwrites_for(self._verified_role) + if isinstance(channel, TextChannel): + overwrite = channel.overwrites_for(self._verified_msg_role) + else: + overwrite = channel.overwrites_for(self._verified_voice_role) + if prev_overwrites is None: log.info(f"Missing previous overwrites for #{channel} ({channel.id}); defaulting to None.") - overwrite.update(send_messages=None, add_reactions=None) + overwrite.update(send_messages=None, add_reactions=None, speak=None) else: overwrite.update(**json.loads(prev_overwrites)) - await channel.set_permissions(self._verified_role, overwrite=overwrite) + if isinstance(channel, TextChannel): + await channel.set_permissions(self._verified_msg_role, overwrite=overwrite) + else: + await channel.set_permissions(self._verified_voice_role, overwrite=overwrite) log.info(f"Unsilenced channel #{channel} ({channel.id}).") self.scheduler.cancel(channel.id) @@ -204,11 +280,18 @@ class Silence(commands.Cog): await self.unsilence_timestamps.delete(channel.id) if prev_overwrites is None: - await self._mod_alerts_channel.send( - f"<@&{Roles.admins}> Restored overwrites with default values after unsilencing " - f"{channel.mention}. Please check that the `Send Messages` and `Add Reactions` " - f"overwrites for {self._verified_role.mention} are at their desired values." - ) + if isinstance(channel, TextChannel): + await self._mod_alerts_channel.send( + f"<@&{Roles.admins}> Restored overwrites with default values after unsilencing " + f"{channel.mention}. Please check that the `Send Messages` and `Add Reactions` " + f"overwrites for {self._verified_msg_role.mention} are at their desired values." + ) + else: + await self._mod_alerts_channel.send( + f"<@&{Roles.admins}> Restored overwrites with default values after unsilencing " + f"{channel.mention}. Please check that the `Speak` " + f"overwrites for {self._verified_voice_role.mention} are at their desired values." + ) return True diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 104293d8e..bac933115 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -123,7 +123,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): guild.get_role.side_effect = lambda id_: Mock(id=id_) await self.cog._async_init() - self.assertEqual(self.cog._verified_role.id, Roles.verified) + self.assertEqual(self.cog._verified_msg_role.id, Roles.verified) @autospec(silence, "SilenceNotifier", pass_mocks=False) async def test_async_init_got_channels(self): @@ -277,7 +277,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=was_silenced): with self.subTest(was_silenced=was_silenced, message=message, duration=duration): await self.cog.silence.callback(self.cog, ctx, duration) - ctx.send.assert_called_once_with(message) + ctx.channel.send.assert_called_once_with(message) async def test_skipped_already_silenced(self): """Permissions were not set and `False` was returned for an already silenced channel.""" @@ -302,7 +302,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.assertFalse(self.overwrite.send_messages) self.assertFalse(self.overwrite.add_reactions) self.channel.set_permissions.assert_awaited_once_with( - self.cog._verified_role, + self.cog._verified_msg_role, overwrite=self.overwrite ) @@ -372,7 +372,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): args = (300, ctx.channel.id, ctx.invoke.return_value) self.cog.scheduler.schedule_later.assert_called_once_with(*args) - ctx.invoke.assert_called_once_with(self.cog.unsilence) + ctx.invoke.assert_called_once_with(self.cog.unsilence, channel=ctx.channel) async def test_permanent_not_scheduled(self): """A task was not scheduled for a permanent silence.""" @@ -435,7 +435,7 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): """Channel's `send_message` and `add_reactions` overwrites were restored.""" await self.cog._unsilence(self.channel) self.channel.set_permissions.assert_awaited_once_with( - self.cog._verified_role, + self.cog._verified_msg_role, overwrite=self.overwrite, ) @@ -449,7 +449,7 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence(self.channel) self.channel.set_permissions.assert_awaited_once_with( - self.cog._verified_role, + self.cog._verified_msg_role, overwrite=self.overwrite, ) -- cgit v1.2.3 From 51daa10a98299c5bcf4455b34e10c35850474521 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sun, 22 Nov 2020 12:50:58 +0300 Subject: Improves Channel Converter Usability - Allows spaces in channel name - Allows channel name to have any capitalization - Fixed inherited class to general Converter class Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/converters.py | 8 ++++---- bot/exts/moderation/silence.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index b9db37fce..613be73eb 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -536,7 +536,7 @@ class FetchedUser(UserConverter): raise BadArgument(f"User `{arg}` does not exist") -class AnyChannelConverter(UserConverter): +class AnyChannelConverter(Converter): """ Converts to a `discord.Channel` or, raises an error. @@ -557,15 +557,15 @@ class AnyChannelConverter(UserConverter): async def convert(self, ctx: Context, arg: str) -> t.Union[discord.TextChannel, discord.VoiceChannel]: """Convert the `arg` to a `TextChannel` or `VoiceChannel`.""" - stripped = arg.strip().lstrip("<").lstrip("#").rstrip(">") + stripped = arg.strip().lstrip("<").lstrip("#").rstrip(">").lower() # Filter channels by name and ID - channels = [channel for channel in ctx.guild.channels if stripped in (channel.name, str(channel.id))] + channels = [channel for channel in ctx.guild.channels if stripped in (channel.name.lower(), str(channel.id))] if len(channels) == 0: # Couldn't find a matching channel log.debug(f"Could not convert `{arg}` to channel, no matches found.") - raise BadArgument("The provided argument returned no matches.") + raise BadArgument(f"{arg} returned no matches.") elif len(channels) > 1: # Couldn't discern the desired channel diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 266669eed..c903bfe9e 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -141,7 +141,7 @@ class Silence(commands.Cog): @commands.command(aliases=("hush",)) @lock_arg(LOCK_NAMESPACE, "ctx", attrgetter("channel"), raise_error=True) async def silence(self, ctx: Context, duration: HushDurationConverter = 10, - channel: AnyChannelConverter = None) -> None: + *, channel: AnyChannelConverter = None) -> None: """ Silence the current channel for `duration` minutes or `forever`. @@ -171,7 +171,7 @@ class Silence(commands.Cog): await self.send_message(MSG_SILENCE_SUCCESS, ctx.channel, channel, True, duration) @commands.command(aliases=("unhush",)) - async def unsilence(self, ctx: Context, channel: AnyChannelConverter = None) -> None: + async def unsilence(self, ctx: Context, *, channel: AnyChannelConverter = None) -> None: """ Unsilence the given channel if given, else the current one. -- cgit v1.2.3 From cf56c22b048567b7782aeff123f3df0eee621cea Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sun, 22 Nov 2020 12:53:29 +0300 Subject: Refactor Silence Class Imports Refactors imports of silence class to be more inline with the original import structure. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index c903bfe9e..314aa946e 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -1,10 +1,9 @@ import json import logging -import typing from contextlib import suppress from datetime import datetime, timedelta, timezone from operator import attrgetter -from typing import Optional +from typing import Optional, Union from async_rediscache import RedisCache from discord import TextChannel, VoiceChannel @@ -13,7 +12,7 @@ from discord.ext.commands import Context from bot.bot import Bot from bot.constants import Channels, Emojis, Guild, MODERATION_ROLES, Roles -from bot.converters import HushDurationConverter, AnyChannelConverter +from bot.converters import AnyChannelConverter, HushDurationConverter from bot.utils.lock import LockedResourceError, lock_arg from bot.utils.scheduling import Scheduler @@ -42,7 +41,7 @@ class SilenceNotifier(tasks.Loop): self._silenced_channels = {} self._alert_channel = alert_channel - def add_channel(self, channel: typing.Union[TextChannel, VoiceChannel]) -> None: + def add_channel(self, channel: Union[TextChannel, VoiceChannel]) -> None: """Add channel to `_silenced_channels` and start loop if not launched.""" if not self._silenced_channels: self.start() @@ -101,9 +100,9 @@ class Silence(commands.Cog): await self._reschedule() async def send_message(self, message: str, source_channel: TextChannel, - target_channel: typing.Union[TextChannel, VoiceChannel], + target_channel: Union[TextChannel, VoiceChannel], alert_target: bool = False, duration: HushDurationConverter = 0) -> None: - """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`""" + """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`.""" if isinstance(source_channel, TextChannel): await source_channel.send( message.replace("current", target_channel.mention if source_channel != target_channel else "current") @@ -184,8 +183,8 @@ class Silence(commands.Cog): await self._unsilence_wrapper(channel, ctx) @lock_arg(LOCK_NAMESPACE, "channel", raise_error=True) - async def _unsilence_wrapper(self, channel: typing.Union[TextChannel, VoiceChannel], - ctx: typing.Optional[Context] = None) -> None: + async def _unsilence_wrapper(self, channel: Union[TextChannel, VoiceChannel], + ctx: Optional[Context] = None) -> None: """Unsilence `channel` and send a success/failure message.""" msg_channel = channel if ctx is not None: @@ -209,7 +208,7 @@ class Silence(commands.Cog): # Send success message to muted channel or voice chat channel, and invocation channel await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, True) - async def _set_silence_overwrites(self, channel: typing.Union[TextChannel, VoiceChannel]) -> bool: + async def _set_silence_overwrites(self, channel: Union[TextChannel, VoiceChannel]) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" if isinstance(channel, TextChannel): overwrite = channel.overwrites_for(self._verified_msg_role) @@ -242,7 +241,7 @@ class Silence(commands.Cog): unsilence_time = datetime.now(tz=timezone.utc) + timedelta(minutes=duration) await self.unsilence_timestamps.set(channel.id, unsilence_time.timestamp()) - async def _unsilence(self, channel: typing.Union[TextChannel, VoiceChannel]) -> bool: + async def _unsilence(self, channel: Union[TextChannel, VoiceChannel]) -> bool: """ Unsilence `channel`. -- cgit v1.2.3 From 06f9c48e136644a15044b84a0b25f1b6feba0e09 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sun, 22 Nov 2020 13:07:46 +0300 Subject: Add VC Mute Functionality Adds and calls a function to force a voice channel member to sync permissions. See #1160 for why this is necessary. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 66 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 314aa946e..415ab19a6 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -6,7 +6,7 @@ from operator import attrgetter from typing import Optional, Union from async_rediscache import RedisCache -from discord import TextChannel, VoiceChannel +from discord import HTTPException, Member, PermissionOverwrite, TextChannel, VoiceChannel from discord.ext import commands, tasks from discord.ext.commands import Context @@ -93,9 +93,13 @@ class Silence(commands.Cog): await self.bot.wait_until_guild_available() guild = self.bot.get_guild(Guild.id) + self._verified_msg_role = guild.get_role(Roles.verified) self._verified_voice_role = guild.get_role(Roles.voice_verified) + self._helper_role = guild.get_role(Roles.helpers) + self._mod_alerts_channel = self.bot.get_channel(Channels.mod_alerts) + self.notifier = SilenceNotifier(self.bot.get_channel(Channels.mod_log)) await self._reschedule() @@ -226,12 +230,70 @@ class Silence(commands.Cog): else: overwrite.update(speak=False) await channel.set_permissions(self._verified_voice_role, overwrite=overwrite) + try: + await self._force_voice_silence(channel) + except HTTPException: + # TODO: Relay partial failure to invocation channel + pass await self.previous_overwrites.set(channel.id, json.dumps(prev_overwrites)) return True - async def _schedule_unsilence(self, ctx: Context, channel: typing.Union[TextChannel, VoiceChannel], + async def _force_voice_silence(self, channel: VoiceChannel, member: Optional[Member] = None) -> None: + """ + Move all non-staff members from `channel` to a temporary channel and back to force toggle role mute. + + If `member` is passed, the mute only occurs to that member. + Permission modification has to happen before this function. + + Raises `discord.HTTPException` if the task fails. + """ + # Obtain temporary channel + afk_channel = channel.guild.afk_channel + if afk_channel is None: + try: + overwrites = { + channel.guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) + } + afk_channel = await channel.guild.create_voice_channel("mute-temp", overwrites=overwrites) + log.info(f"Failed to get afk-channel, created temporary channel #{afk_channel} ({afk_channel.id})") + + # Schedule channel deletion in case function errors out + self.scheduler.schedule_later(30, afk_channel.id, + afk_channel.delete(reason="Deleting temp mute channel.")) + + except HTTPException as e: + log.warning("Failed to create temporary mute channel.", exc_info=e) + raise e + + # Handle member picking logic + if member is not None: + members = [member] + else: + members = channel.members + + # Move all members to temporary channel and back + for member in members: + # Skip staff + if self._helper_role in member.roles: + continue + + try: + await member.move_to(afk_channel, reason="Muting member.") + log.debug(f"Moved {member.name} to afk channel.") + + await member.move_to(channel, reason="Muting member.") + log.debug(f"Moved {member.name} to original voice channel.") + + except HTTPException as e: + log.warning(f"Failed to move {member.name} while muting, falling back to kick.", exc_info=e) + try: + await member.move_to(None, reason="Forcing member mute.") + except HTTPException: + pass + + async def _schedule_unsilence(self, ctx: Context, channel: Union[TextChannel, VoiceChannel], duration: Optional[int]) -> None: """Schedule `ctx.channel` to be unsilenced if `duration` is not None.""" if duration is None: -- cgit v1.2.3 From 5cfc0aca6cbe263259b6b25d6828f73f744d0661 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 00:19:09 +0300 Subject: Add VC Mute Failure Notification Notifies invocation channel that the silence command failed to silence the channel because it could not move members, but roles were updated. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 45 ++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 415ab19a6..9020634f4 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -23,10 +23,11 @@ LOCK_NAMESPACE = "silence" MSG_SILENCE_FAIL = f"{Emojis.cross_mark} current channel is already silenced." MSG_SILENCE_PERMANENT = f"{Emojis.check_mark} silenced current channel indefinitely." MSG_SILENCE_SUCCESS = f"{Emojis.check_mark} silenced current channel for {{duration}} minute(s)." +MSG_SILENCE_PERM_FAIL = f"{Emojis.cross_mark} failed to force-mute members, permissions updated." MSG_UNSILENCE_FAIL = f"{Emojis.cross_mark} current channel was not silenced." MSG_UNSILENCE_MANUAL = ( - f"{Emojis.cross_mark} current channel was not unsilenced because the current overwrites were " + f"{Emojis.cross_mark} current channel was not unsilenced because the channel overwrites were " f"set manually or the cache was prematurely cleared. " f"Please edit the overwrites manually to unsilence." ) @@ -116,18 +117,18 @@ class Silence(commands.Cog): voice_chat = None if isinstance(target_channel, VoiceChannel): # Send to relevant channel - # TODO: Figure out a non-hardcoded way of doing this - if "offtopic" in target_channel.name.lower(): - voice_chat = self.bot.get_channel(Channels.voice_chat) - elif "code-help" in target_channel.name.lower(): - if "1" in target_channel.name.lower(): - voice_chat = self.bot.get_channel(Channels.code_help_voice) - else: - voice_chat = self.bot.get_channel(Channels.code_help_voice_2) - elif "admin" in target_channel.name.lower(): - voice_chat = self.bot.get_channel(Channels.admins_voice) - elif "staff" in target_channel.name.lower(): - voice_chat = self.bot.get_channel(Channels.staff_voice) + # TODO: Figure out a dynamic way of doing this + channels = { + "offtopic": Channels.voice_chat, + "code/help 1": Channels.code_help_voice, + "code/help 2": Channels.code_help_voice, + "admin": Channels.admins_voice, + "staff": Channels.staff_voice + } + for name in channels.keys(): + if name in target_channel.name.lower(): + voice_chat = self.bot.get_channel(channels[name]) + break if alert_target and source_channel != target_channel: if isinstance(target_channel, VoiceChannel): @@ -157,9 +158,15 @@ class Silence(commands.Cog): channel_info = f"#{channel} ({channel.id})" log.debug(f"{ctx.author} is silencing channel {channel_info}.") - if not await self._set_silence_overwrites(channel): - log.info(f"Tried to silence channel {channel_info} but the channel was already silenced.") - await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel) + try: + if not await self._set_silence_overwrites(channel): + log.info(f"Tried to silence channel {channel_info} but the channel was already silenced.") + await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel) + return + + except HTTPException: + log.info(f"Could not force mute {channel_info} members. Permissions updated.") + await self.send_message(MSG_SILENCE_PERM_FAIL, ctx.channel, channel) return await self._schedule_unsilence(ctx, channel, duration) @@ -230,11 +237,7 @@ class Silence(commands.Cog): else: overwrite.update(speak=False) await channel.set_permissions(self._verified_voice_role, overwrite=overwrite) - try: - await self._force_voice_silence(channel) - except HTTPException: - # TODO: Relay partial failure to invocation channel - pass + await self._force_voice_silence(channel) await self.previous_overwrites.set(channel.id, json.dumps(prev_overwrites)) -- cgit v1.2.3 From 2f57f40d3bfdc3b5c6cdb9e37fffd101cf195b12 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 01:24:15 +0300 Subject: Write AnyChannelConverter Tests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/test_converters.py | 48 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index c42111f3f..5039d45ea 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -7,6 +7,7 @@ from dateutil.relativedelta import relativedelta from discord.ext.commands import BadArgument from bot.converters import ( + AnyChannelConverter, Duration, HushDurationConverter, ISODateTime, @@ -24,6 +25,18 @@ class ConverterTests(unittest.IsolatedAsyncioTestCase): cls.context = MagicMock cls.context.author = 'bob' + cls.context.guild = MagicMock + cls.context.guild.channels = [] + + for i in range(10): + channel = MagicMock() + channel.name = f"test-{i}" + channel.id = str(i) * 18 + + cls.context.guild.channels.append(channel) + if i > 7: + cls.context.guild.channels.append(channel) + cls.fixed_utc_now = datetime.datetime.fromisoformat('2019-01-01T00:00:00') async def test_tag_content_converter_for_valid(self): @@ -312,3 +325,38 @@ class ConverterTests(unittest.IsolatedAsyncioTestCase): with self.subTest(invalid_minutes_string=invalid_minutes_string, exception_message=exception_message): with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): await converter.convert(self.context, invalid_minutes_string) + + async def test_any_channel_converter_for_valid(self): + """AnyChannelConverter returns correct channel from input.""" + test_values = ( + ("test-0", self.context.guild.channels[0]), + ("#test-0", self.context.guild.channels[0]), + (" #tEsT-0 ", self.context.guild.channels[0]), + (f"<#{'0' * 18}>", self.context.guild.channels[0]), + (f"{'0' * 18}", self.context.guild.channels[0]), + ("test-5", self.context.guild.channels[5]), + ("#test-5", self.context.guild.channels[5]), + (f"<#{'5' * 18}>", self.context.guild.channels[5]), + (f"{'5' * 18}", self.context.guild.channels[5]), + ) + + converter = AnyChannelConverter() + for input_string, expected_channel in test_values: + with self.subTest(input_string=input_string, expected_channel=expected_channel): + converted = await converter.convert(self.context, input_string) + self.assertEqual(expected_channel, converted) + + async def test_any_channel_converter_for_invalid(self): + """AnyChannelConverter raises BadArgument for invalid channels.""" + test_values = ( + ("#test-8", "The provided argument returned too many matches (2)."), + ("#test-9", "The provided argument returned too many matches (2)."), + ("#random-name", "#random-name returned no matches."), + ("test-10", "test-10 returned no matches.") + ) + + converter = AnyChannelConverter() + for invalid_input, exception_message in test_values: + with self.subTest(invalid_input=invalid_input, exception_message=exception_message): + with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): + await converter.convert(self.context, invalid_input) \ No newline at end of file -- cgit v1.2.3 From 81f923ed784cf09e3b7e90e57a9ad0111099619c Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 01:24:15 +0300 Subject: Write AnyChannelConverter Tests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/test_converters.py | 48 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index c42111f3f..6bea71977 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -7,6 +7,7 @@ from dateutil.relativedelta import relativedelta from discord.ext.commands import BadArgument from bot.converters import ( + AnyChannelConverter, Duration, HushDurationConverter, ISODateTime, @@ -24,6 +25,18 @@ class ConverterTests(unittest.IsolatedAsyncioTestCase): cls.context = MagicMock cls.context.author = 'bob' + cls.context.guild = MagicMock + cls.context.guild.channels = [] + + for i in range(10): + channel = MagicMock() + channel.name = f"test-{i}" + channel.id = str(i) * 18 + + cls.context.guild.channels.append(channel) + if i > 7: + cls.context.guild.channels.append(channel) + cls.fixed_utc_now = datetime.datetime.fromisoformat('2019-01-01T00:00:00') async def test_tag_content_converter_for_valid(self): @@ -312,3 +325,38 @@ class ConverterTests(unittest.IsolatedAsyncioTestCase): with self.subTest(invalid_minutes_string=invalid_minutes_string, exception_message=exception_message): with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): await converter.convert(self.context, invalid_minutes_string) + + async def test_any_channel_converter_for_valid(self): + """AnyChannelConverter returns correct channel from input.""" + test_values = ( + ("test-0", self.context.guild.channels[0]), + ("#test-0", self.context.guild.channels[0]), + (" #tEsT-0 ", self.context.guild.channels[0]), + (f"<#{'0' * 18}>", self.context.guild.channels[0]), + (f"{'0' * 18}", self.context.guild.channels[0]), + ("test-5", self.context.guild.channels[5]), + ("#test-5", self.context.guild.channels[5]), + (f"<#{'5' * 18}>", self.context.guild.channels[5]), + (f"{'5' * 18}", self.context.guild.channels[5]), + ) + + converter = AnyChannelConverter() + for input_string, expected_channel in test_values: + with self.subTest(input_string=input_string, expected_channel=expected_channel): + converted = await converter.convert(self.context, input_string) + self.assertEqual(expected_channel, converted) + + async def test_any_channel_converter_for_invalid(self): + """AnyChannelConverter raises BadArgument for invalid channels.""" + test_values = ( + ("#test-8", "The provided argument returned too many matches (2)."), + ("#test-9", "The provided argument returned too many matches (2)."), + ("#random-name", "#random-name returned no matches."), + ("test-10", "test-10 returned no matches.") + ) + + converter = AnyChannelConverter() + for invalid_input, exception_message in test_values: + with self.subTest(invalid_input=invalid_input, exception_message=exception_message): + with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): + await converter.convert(self.context, invalid_input) -- cgit v1.2.3 From 985b681b0a5679fb9816e961ffb2abd69ef305bf Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 09:20:41 +0300 Subject: Make Voice Channel Kick Optional Adds an optional parameter to the silence command to enable moderators to choose if they only update permissions, or kick members too. As an accompanying feature, the unsilence command now syncs voice channel permissions too. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 50 ++++++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 9020634f4..93a0dad98 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -144,13 +144,16 @@ class Silence(commands.Cog): @commands.command(aliases=("hush",)) @lock_arg(LOCK_NAMESPACE, "ctx", attrgetter("channel"), raise_error=True) - async def silence(self, ctx: Context, duration: HushDurationConverter = 10, - *, channel: AnyChannelConverter = None) -> None: + async def silence(self, ctx: Context, duration: HushDurationConverter = 10, kick: bool = False, + *, channel: Optional[AnyChannelConverter] = None) -> None: """ 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. + + Passing a voice channel will attempt to move members out of the channel and back to force sync permissions. + If `kick` is True, members will not be added back to the voice channel, and members will be unable to rejoin. """ await self._init_task if channel is None: @@ -159,7 +162,7 @@ class Silence(commands.Cog): log.debug(f"{ctx.author} is silencing channel {channel_info}.") try: - if not await self._set_silence_overwrites(channel): + if not await self._set_silence_overwrites(channel, kick): log.info(f"Tried to silence channel {channel_info} but the channel was already silenced.") await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel) return @@ -217,9 +220,10 @@ class Silence(commands.Cog): else: # Send success message to muted channel or voice chat channel, and invocation channel + await self._force_voice_sync(channel) await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, True) - async def _set_silence_overwrites(self, channel: Union[TextChannel, VoiceChannel]) -> bool: + async def _set_silence_overwrites(self, channel: Union[TextChannel, VoiceChannel], kick: bool) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" if isinstance(channel, TextChannel): overwrite = channel.overwrites_for(self._verified_msg_role) @@ -227,6 +231,8 @@ class Silence(commands.Cog): else: overwrite = channel.overwrites_for(self._verified_voice_role) prev_overwrites = dict(speak=overwrite.speak) + if kick: + prev_overwrites.update(connect=overwrite.connect) if channel.id in self.scheduler or all(val is False for val in prev_overwrites.values()): return False @@ -236,22 +242,43 @@ class Silence(commands.Cog): await channel.set_permissions(self._verified_msg_role, overwrite=overwrite) else: overwrite.update(speak=False) + if kick: + overwrite.update(connect=False) + await channel.set_permissions(self._verified_voice_role, overwrite=overwrite) - await self._force_voice_silence(channel) + + await self._force_voice_sync(channel, kick=kick) await self.previous_overwrites.set(channel.id, json.dumps(prev_overwrites)) return True - async def _force_voice_silence(self, channel: VoiceChannel, member: Optional[Member] = None) -> None: + async def _force_voice_sync(self, channel: VoiceChannel, member: Optional[Member] = None, + kick: bool = False) -> None: """ Move all non-staff members from `channel` to a temporary channel and back to force toggle role mute. If `member` is passed, the mute only occurs to that member. Permission modification has to happen before this function. + If `kick_all` is True, members will not be added back to the voice channel + Raises `discord.HTTPException` if the task fails. """ + # Handle member picking logic + if member is not None: + members = [member] + else: + members = channel.members + + # Handle kick logic + if kick: + for member in members: + await member.move_to(None, reason="Kicking voice channel member.") + + log.debug(f"Kicked all members from #{channel.name} ({channel.id}).") + return + # Obtain temporary channel afk_channel = channel.guild.afk_channel if afk_channel is None: @@ -270,12 +297,6 @@ class Silence(commands.Cog): log.warning("Failed to create temporary mute channel.", exc_info=e) raise e - # Handle member picking logic - if member is not None: - members = [member] - else: - members = channel.members - # Move all members to temporary channel and back for member in members: # Skip staff @@ -291,10 +312,7 @@ class Silence(commands.Cog): except HTTPException as e: log.warning(f"Failed to move {member.name} while muting, falling back to kick.", exc_info=e) - try: - await member.move_to(None, reason="Forcing member mute.") - except HTTPException: - pass + await member.move_to(None, reason="Forcing member mute.") async def _schedule_unsilence(self, ctx: Context, channel: Union[TextChannel, VoiceChannel], duration: Optional[int]) -> None: -- cgit v1.2.3 From 02be861e90c176ae7b577829f4fc636215cdcc3c Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 09:26:01 +0300 Subject: Fix Failing Functions Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 93a0dad98..87327e72a 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -223,7 +223,7 @@ class Silence(commands.Cog): await self._force_voice_sync(channel) await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, True) - async def _set_silence_overwrites(self, channel: Union[TextChannel, VoiceChannel], kick: bool) -> bool: + async def _set_silence_overwrites(self, channel: Union[TextChannel, VoiceChannel], kick: bool = False) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" if isinstance(channel, TextChannel): overwrite = channel.overwrites_for(self._verified_msg_role) -- cgit v1.2.3 From 7569fefee98e17922857bc3aa4bdcb806c76874b Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 14:00:18 +0300 Subject: Fixes Voice Silence Reporting Fixes the channel reported as muted to voice channel chat channels when silencing voice channels. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 87327e72a..2d928182a 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -108,11 +108,10 @@ class Silence(commands.Cog): target_channel: Union[TextChannel, VoiceChannel], alert_target: bool = False, duration: HushDurationConverter = 0) -> None: """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`.""" - if isinstance(source_channel, TextChannel): - await source_channel.send( - message.replace("current", target_channel.mention if source_channel != target_channel else "current") - .replace("{duration}", str(duration)) - ) + await source_channel.send( + message.replace("current", target_channel.mention if source_channel != target_channel else "current") + .replace("{duration}", str(duration)) + ) voice_chat = None if isinstance(target_channel, VoiceChannel): @@ -136,7 +135,7 @@ class Silence(commands.Cog): return await voice_chat.send( - message.replace("{duration}", str(duration)).replace("current", voice_chat.mention) + message.replace("{duration}", str(duration)).replace("current", target_channel.mention) ) else: -- cgit v1.2.3 From 483114bc29da713870346c0e4b88d008f6281185 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 14:24:27 +0300 Subject: General Silence Class Tests Adds tests for helper functions in the silence cog. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 27 +++++---- tests/bot/exts/moderation/test_silence.py | 91 ++++++++++++++++++++++++++++++- tests/helpers.py | 25 ++++++++- 3 files changed, 126 insertions(+), 17 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 2d928182a..2aebee9d7 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -104,6 +104,20 @@ class Silence(commands.Cog): self.notifier = SilenceNotifier(self.bot.get_channel(Channels.mod_log)) await self._reschedule() + async def _get_related_text_channel(self, channel: VoiceChannel) -> Optional[TextChannel]: + """Returns the text channel related to a voice channel.""" + # TODO: Figure out a dynamic way of doing this + channels = { + "off-topic": Channels.voice_chat, + "code/help 1": Channels.code_help_voice, + "code/help 2": Channels.code_help_voice, + "admin": Channels.admins_voice, + "staff": Channels.staff_voice + } + for name in channels.keys(): + if name in channel.name.lower(): + return self.bot.get_channel(channels[name]) + async def send_message(self, message: str, source_channel: TextChannel, target_channel: Union[TextChannel, VoiceChannel], alert_target: bool = False, duration: HushDurationConverter = 0) -> None: @@ -116,18 +130,7 @@ class Silence(commands.Cog): voice_chat = None if isinstance(target_channel, VoiceChannel): # Send to relevant channel - # TODO: Figure out a dynamic way of doing this - channels = { - "offtopic": Channels.voice_chat, - "code/help 1": Channels.code_help_voice, - "code/help 2": Channels.code_help_voice, - "admin": Channels.admins_voice, - "staff": Channels.staff_voice - } - for name in channels.keys(): - if name in target_channel.name.lower(): - voice_chat = self.bot.get_channel(channels[name]) - break + voice_chat = await self._get_related_text_channel(target_channel) if alert_target and source_channel != target_channel: if isinstance(target_channel, VoiceChannel): diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index bac933115..577725071 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -7,9 +7,9 @@ from unittest.mock import Mock from async_rediscache import RedisSession from discord import PermissionOverwrite -from bot.constants import Channels, Guild, Roles +from bot.constants import Channels, Emojis, Guild, Roles from bot.exts.moderation import silence -from tests.helpers import MockBot, MockContext, MockTextChannel, autospec +from tests.helpers import MockBot, MockContext, MockTextChannel, MockVoiceChannel, autospec redis_session = None redis_loop = asyncio.get_event_loop() @@ -168,6 +168,93 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): role_check.assert_called_once_with(*(1, 2, 3)) role_check.return_value.predicate.assert_awaited_once_with(ctx) + @mock.patch.object(silence.Silence, "_get_related_text_channel") + async def test_send_message(self, mock_get_related_text_channel): + """Test the send function reports to the correct channels.""" + text_channel_1 = MockTextChannel() + text_channel_2 = MockTextChannel() + + voice_channel = MockVoiceChannel() + voice_channel.name = "General/Offtopic" + voice_channel.mention = f"#{voice_channel.name}" + + mock_get_related_text_channel.return_value = text_channel_2 + + def reset(): + text_channel_1.reset_mock() + text_channel_2.reset_mock() + voice_channel.reset_mock() + + with self.subTest("Basic One Channel Test"): + await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, False) + text_channel_1.send.assert_called_once_with("Text basic message.") + text_channel_2.send.assert_not_called() + + reset() + with self.subTest("Basic Two Channel Test"): + await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, True) + text_channel_1.send.assert_called_once_with("Text basic message.") + text_channel_2.send.assert_called_once_with("Text basic message.") + + reset() + with self.subTest("Replacement One Channel Test"): + await self.cog.send_message("The following should be replaced: current", + text_channel_1, text_channel_2, False) + text_channel_1.send.assert_called_once_with(f"The following should be replaced: {text_channel_1.mention}") + text_channel_2.send.assert_not_called() + + reset() + with self.subTest("Replacement Two Channel Test"): + await self.cog.send_message("The following should be replaced: current", + text_channel_1, text_channel_2, True) + text_channel_1.send.assert_called_once_with(f"The following should be replaced: {text_channel_1.mention}") + text_channel_2.send.assert_called_once_with("The following should be replaced: current") + + reset() + with self.subTest("Replace Duration"): + await self.cog.send_message(f"{Emojis.check_mark} The following should be replaced: {{duration}}", + text_channel_1, text_channel_2, False) + text_channel_1.send.assert_called_once_with(f"{Emojis.check_mark} The following should be replaced: 0") + text_channel_2.send.assert_not_called() + + reset() + with self.subTest("Text and Voice"): + await self.cog.send_message("This should show up just here", + text_channel_1, voice_channel, False) + text_channel_1.send.assert_called_once_with("This should show up just here") + + reset() + with self.subTest("Text and Voice"): + await self.cog.send_message("This should show up as current", + text_channel_1, voice_channel, True) + text_channel_1.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") + text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") + + reset() + with self.subTest("Text and Voice Same Invocation"): + await self.cog.send_message("This should show up as current", + text_channel_2, voice_channel, True) + text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") + + async def test_get_related_text_channel(self): + voice_channel = MockVoiceChannel() + + tests = ( + ("Off-Topic/General", Channels.voice_chat), + ("code/help 1", Channels.code_help_voice), + ("Staff", Channels.staff_voice), + ("ADMIN", Channels.admins_voice), + ("not in the channel list", None) + ) + + with mock.patch.object(self.cog.bot, "get_channel", lambda x: x): + for (name, channel_id) in tests: + voice_channel.name = name + voice_channel.id = channel_id + + result_id = await self.cog._get_related_text_channel(voice_channel) + self.assertEqual(result_id, channel_id) + @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) class RescheduleTests(unittest.IsolatedAsyncioTestCase): diff --git a/tests/helpers.py b/tests/helpers.py index 870f66197..5628ca31f 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -16,7 +16,6 @@ from bot.async_stats import AsyncStatsClient from bot.bot import Bot from tests._autospec import autospec # noqa: F401 other modules import it via this module - for logger in logging.Logger.manager.loggerDict.values(): # Set all loggers to CRITICAL by default to prevent screen clutter during testing @@ -320,7 +319,10 @@ channel_data = { } state = unittest.mock.MagicMock() guild = unittest.mock.MagicMock() -channel_instance = discord.TextChannel(state=state, guild=guild, data=channel_data) +text_channel_instance = discord.TextChannel(state=state, guild=guild, data=channel_data) + +channel_data["type"] = "VoiceChannel" +voice_channel_instance = discord.VoiceChannel(state=state, guild=guild, data=channel_data) class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): @@ -330,7 +332,24 @@ class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): Instances of this class will follow the specifications of `discord.TextChannel` instances. For more information, see the `MockGuild` docstring. """ - spec_set = channel_instance + spec_set = text_channel_instance + + def __init__(self, **kwargs) -> None: + default_kwargs = {'id': next(self.discord_id), 'name': 'channel', 'guild': MockGuild()} + super().__init__(**collections.ChainMap(kwargs, default_kwargs)) + + if 'mention' not in kwargs: + self.mention = f"#{self.name}" + + +class MockVoiceChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): + """ + A MagicMock subclass to mock VoiceChannel objects. + + Instances of this class will follow the specifications of `discord.VoiceChannel` instances. For + more information, see the `MockGuild` docstring. + """ + spec_set = voice_channel_instance def __init__(self, **kwargs) -> None: default_kwargs = {'id': next(self.discord_id), 'name': 'channel', 'guild': MockGuild()} -- cgit v1.2.3 From 198dceda779df1372491057d3ef640f390e849e4 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 18:24:24 +0300 Subject: Removes Redundant Exception Handling Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 58 +++++++++++++++--------------------------- 1 file changed, 20 insertions(+), 38 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 2aebee9d7..c60ca9327 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -6,7 +6,7 @@ from operator import attrgetter from typing import Optional, Union from async_rediscache import RedisCache -from discord import HTTPException, Member, PermissionOverwrite, TextChannel, VoiceChannel +from discord import Member, PermissionOverwrite, TextChannel, VoiceChannel from discord.ext import commands, tasks from discord.ext.commands import Context @@ -23,7 +23,6 @@ LOCK_NAMESPACE = "silence" MSG_SILENCE_FAIL = f"{Emojis.cross_mark} current channel is already silenced." MSG_SILENCE_PERMANENT = f"{Emojis.check_mark} silenced current channel indefinitely." MSG_SILENCE_SUCCESS = f"{Emojis.check_mark} silenced current channel for {{duration}} minute(s)." -MSG_SILENCE_PERM_FAIL = f"{Emojis.cross_mark} failed to force-mute members, permissions updated." MSG_UNSILENCE_FAIL = f"{Emojis.cross_mark} current channel was not silenced." MSG_UNSILENCE_MANUAL = ( @@ -163,15 +162,9 @@ class Silence(commands.Cog): channel_info = f"#{channel} ({channel.id})" log.debug(f"{ctx.author} is silencing channel {channel_info}.") - try: - if not await self._set_silence_overwrites(channel, kick): - log.info(f"Tried to silence channel {channel_info} but the channel was already silenced.") - await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel) - return - - except HTTPException: - log.info(f"Could not force mute {channel_info} members. Permissions updated.") - await self.send_message(MSG_SILENCE_PERM_FAIL, ctx.channel, channel) + if not await self._set_silence_overwrites(channel, kick): + log.info(f"Tried to silence channel {channel_info} but the channel was already silenced.") + await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel) return await self._schedule_unsilence(ctx, channel, duration) @@ -222,7 +215,9 @@ class Silence(commands.Cog): else: # Send success message to muted channel or voice chat channel, and invocation channel - await self._force_voice_sync(channel) + if isinstance(channel, VoiceChannel): + await self._force_voice_sync(channel) + await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, True) async def _set_silence_overwrites(self, channel: Union[TextChannel, VoiceChannel], kick: bool = False) -> bool: @@ -248,7 +243,6 @@ class Silence(commands.Cog): overwrite.update(connect=False) await channel.set_permissions(self._verified_voice_role, overwrite=overwrite) - await self._force_voice_sync(channel, kick=kick) await self.previous_overwrites.set(channel.id, json.dumps(prev_overwrites)) @@ -263,9 +257,7 @@ class Silence(commands.Cog): If `member` is passed, the mute only occurs to that member. Permission modification has to happen before this function. - If `kick_all` is True, members will not be added back to the voice channel - - Raises `discord.HTTPException` if the task fails. + If `kick_all` is True, members will not be added back to the voice channel. """ # Handle member picking logic if member is not None: @@ -284,20 +276,15 @@ class Silence(commands.Cog): # Obtain temporary channel afk_channel = channel.guild.afk_channel if afk_channel is None: - try: - overwrites = { - channel.guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) - } - afk_channel = await channel.guild.create_voice_channel("mute-temp", overwrites=overwrites) - log.info(f"Failed to get afk-channel, created temporary channel #{afk_channel} ({afk_channel.id})") - - # Schedule channel deletion in case function errors out - self.scheduler.schedule_later(30, afk_channel.id, - afk_channel.delete(reason="Deleting temp mute channel.")) + overwrites = { + channel.guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) + } + afk_channel = await channel.guild.create_voice_channel("mute-temp", overwrites=overwrites) + log.info(f"Failed to get afk-channel, created temporary channel #{afk_channel} ({afk_channel.id})") - except HTTPException as e: - log.warning("Failed to create temporary mute channel.", exc_info=e) - raise e + # Schedule channel deletion in case function errors out + self.scheduler.schedule_later(30, afk_channel.id, + afk_channel.delete(reason="Deleting temp mute channel.")) # Move all members to temporary channel and back for member in members: @@ -305,16 +292,11 @@ class Silence(commands.Cog): if self._helper_role in member.roles: continue - try: - await member.move_to(afk_channel, reason="Muting member.") - log.debug(f"Moved {member.name} to afk channel.") - - await member.move_to(channel, reason="Muting member.") - log.debug(f"Moved {member.name} to original voice channel.") + await member.move_to(afk_channel, reason="Muting member.") + log.debug(f"Moved {member.name} to afk channel.") - except HTTPException as e: - log.warning(f"Failed to move {member.name} while muting, falling back to kick.", exc_info=e) - await member.move_to(None, reason="Forcing member mute.") + await member.move_to(channel, reason="Muting member.") + log.debug(f"Moved {member.name} to original voice channel.") async def _schedule_unsilence(self, ctx: Context, channel: Union[TextChannel, VoiceChannel], duration: Optional[int]) -> None: -- cgit v1.2.3 From 00a26b95bc248e3e7b13263df8c6c0d46ebbb2ff Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 23:09:13 +0300 Subject: Adds Silence & Unsilence UnitTests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 212 +++++++++++++++++++++++++++++- 1 file changed, 211 insertions(+), 1 deletion(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 577725071..1d05ee357 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -9,7 +9,7 @@ from discord import PermissionOverwrite from bot.constants import Channels, Emojis, Guild, Roles from bot.exts.moderation import silence -from tests.helpers import MockBot, MockContext, MockTextChannel, MockVoiceChannel, autospec +from tests.helpers import MockBot, MockContext, MockGuild, MockMember, MockTextChannel, MockVoiceChannel, autospec redis_session = None redis_loop = asyncio.get_event_loop() @@ -237,6 +237,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") async def test_get_related_text_channel(self): + """Tests the helper function that connects voice to text channels.""" voice_channel = MockVoiceChannel() tests = ( @@ -255,6 +256,72 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): result_id = await self.cog._get_related_text_channel(voice_channel) self.assertEqual(result_id, channel_id) + async def test_force_voice_sync(self): + """Tests the _force_voice_sync helper function.""" + await self.cog._async_init() + + afk_channel = MockVoiceChannel() + channel = MockVoiceChannel(guild=MockGuild(afk_channel=afk_channel)) + + members = [] + for _ in range(10): + members.append(MockMember()) + + channel.members = members + test_cases = ( + (members[0], False, "Muting member."), + (members[0], True, "Kicking voice channel member."), + (None, False, "Muting member."), + (None, True, "Kicking voice channel member."), + ) + + for member, kick, reason in test_cases: + with self.subTest(members=member, kick=kick, reason=reason): + await self.cog._force_voice_sync(channel, member, kick) + + for single_member in channel.members if member is None else [member]: + if kick: + single_member.move_to.assert_called_once_with(None, reason=reason) + else: + self.assertEqual(single_member.move_to.call_count, 2) + single_member.move_to.assert_has_calls([ + mock.call(afk_channel, reason=reason), + mock.call(channel, reason=reason) + ], any_order=False) + + single_member.reset_mock() + + async def test_force_voice_sync_staff(self): + """Tests to ensure _force_voice_sync does not kick staff members.""" + await self.cog._async_init() + member = MockMember(roles=[self.cog._helper_role]) + + await self.cog._force_voice_sync(MockVoiceChannel(), member) + member.move_to.assert_not_called() + + async def test_force_voice_sync_no_channel(self): + """Test to ensure _force_voice_sync can create its own voice channel if one is not available.""" + await self.cog._async_init() + + member = MockMember() + channel = MockVoiceChannel(guild=MockGuild(afk_channel=None)) + + new_channel = MockVoiceChannel(delete=Mock()) + channel.guild.create_voice_channel.return_value = new_channel + + with mock.patch.object(self.cog.scheduler, "schedule_later") as scheduler: + await self.cog._force_voice_sync(channel, member) + + # Check channel creation + overwrites = { + channel.guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) + } + channel.guild.create_voice_channel.assert_called_once_with("mute-temp", overwrites=overwrites) + + # Check bot queued deletion + new_channel.delete.assert_called_once_with(reason="Deleting temp mute channel.") + scheduler.assert_called_once_with(30, new_channel.id, new_channel.delete()) + @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) class RescheduleTests(unittest.IsolatedAsyncioTestCase): @@ -366,6 +433,31 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.silence.callback(self.cog, ctx, duration) ctx.channel.send.assert_called_once_with(message) + async def test_sent_to_correct_channel(self): + """Test function sends messages to the correct channels.""" + text_channel = MockTextChannel() + ctx = MockContext() + + test_cases = ( + (None, silence.MSG_SILENCE_SUCCESS.format(duration=10)), + (text_channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), + (ctx.channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), + ) + + for target, message in test_cases: + with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): + with self.subTest(target_channel=target, message=message): + await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) + if ctx.channel == target or target is None: + ctx.channel.send.assert_called_once_with(message) + else: + ctx.channel.send.assert_called_once_with(message.replace("current", text_channel.mention)) + target.send.assert_called_once_with(message) + + ctx.channel.send.reset_mock() + if target is not None: + target.send.reset_mock() + async def test_skipped_already_silenced(self): """Permissions were not set and `False` was returned for an already silenced channel.""" subtests = ( @@ -509,6 +601,40 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.unsilence.callback(self.cog, ctx) ctx.channel.send.assert_called_once_with(message) + async def test_sent_to_correct_channel(self): + """Test function sends messages to the correct channels.""" + unsilenced_overwrite = PermissionOverwrite(send_messages=True, add_reactions=True) + text_channel = MockTextChannel() + ctx = MockContext() + + test_cases = ( + (None, silence.MSG_UNSILENCE_SUCCESS.format(duration=10)), + (text_channel, silence.MSG_UNSILENCE_SUCCESS.format(duration=10)), + (ctx.channel, silence.MSG_UNSILENCE_SUCCESS.format(duration=10)), + ) + + for target, message in test_cases: + with self.subTest(target_channel=target, message=message): + with mock.patch.object(self.cog, "_unsilence", return_value=True): + # Assign Return + if ctx.channel == target or target is None: + ctx.channel.overwrites_for.return_value = unsilenced_overwrite + else: + target.overwrites_for.return_value = unsilenced_overwrite + + await self.cog.unsilence.callback(self.cog, ctx, channel=target) + + # Check Messages + if ctx.channel == target or target is None: + ctx.channel.send.assert_called_once_with(message) + else: + ctx.channel.send.assert_called_once_with(message.replace("current", text_channel.mention)) + target.send.assert_called_once_with(message) + + ctx.channel.send.reset_mock() + if target is not None: + target.send.reset_mock() + async def test_skipped_already_unsilenced(self): """Permissions were not set and `False` was returned for an already unsilenced channel.""" self.cog.scheduler.__contains__.return_value = False @@ -549,6 +675,10 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence(self.channel) self.cog._mod_alerts_channel.send.assert_awaited_once() + self.cog._mod_alerts_channel.send.reset_mock() + + await self.cog._unsilence(MockVoiceChannel()) + self.cog._mod_alerts_channel.send.assert_awaited_once() async def test_removed_notifier(self): """Channel was removed from `notifier`.""" @@ -587,3 +717,83 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): del new_overwrite_dict['add_reactions'] self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) + + async def test_unsilence_helper_fail(self): + """Tests unsilence_wrapper when `_unsilence` fails.""" + ctx = MockContext() + + text_channel = MockTextChannel() + text_role = self.cog.bot.get_guild(Guild.id).get_role(Roles.verified) + + voice_channel = MockVoiceChannel() + voice_role = self.cog.bot.get_guild(Guild.id).get_role(Roles.voice_verified) + + test_cases = ( + (ctx, text_channel, text_role, True, silence.MSG_UNSILENCE_FAIL), + (ctx, text_channel, text_role, False, silence.MSG_UNSILENCE_MANUAL), + (ctx, voice_channel, voice_role, True, silence.MSG_UNSILENCE_FAIL), + (ctx, voice_channel, voice_role, False, silence.MSG_UNSILENCE_MANUAL), + ) + + class PermClass: + """Class to Mock return permissions""" + def __init__(self, value: bool): + self.val = value + + def __getattr__(self, item): + return self.val + + for context, channel, role, permission, message in test_cases: + with self.subTest(channel=channel, message=message): + with mock.patch.object(channel, "overwrites_for", return_value=PermClass(permission)) as overwrites: + with mock.patch.object(self.cog, "send_message") as send_message: + with mock.patch.object(self.cog, "_unsilence", return_value=False): + await self.cog._unsilence_wrapper(channel, context) + + overwrites.assert_called_once_with(role) + send_message.assert_called_once_with(message, ctx.channel, channel) + + async def test_correct_overwrites(self): + """Tests the overwrites returned by the _unsilence_wrapper are correct for voice and text channels.""" + ctx = MockContext() + + text_channel = MockTextChannel() + text_role = self.cog.bot.get_guild(Guild.id).get_role(Roles.verified) + + voice_channel = MockVoiceChannel() + voice_role = self.cog.bot.get_guild(Guild.id).get_role(Roles.voice_verified) + + async def reset(): + await text_channel.set_permissions(text_role, PermissionOverwrite(send_messages=False, add_reactions=False)) + await voice_channel.set_permissions(voice_role, PermissionOverwrite(speak=False, connect=False)) + + text_channel.reset_mock() + voice_channel.reset_mock() + await reset() + + default_text_overwrites = text_channel.overwrites_for(text_role) + default_voice_overwrites = voice_channel.overwrites_for(voice_role) + + test_cases = ( + (ctx, text_channel, text_role, default_text_overwrites, silence.MSG_UNSILENCE_SUCCESS), + (ctx, voice_channel, voice_role, default_voice_overwrites, silence.MSG_UNSILENCE_SUCCESS), + (ctx, ctx.channel, text_role, ctx.channel.overwrites_for(text_role), silence.MSG_UNSILENCE_SUCCESS), + (None, text_channel, text_role, default_text_overwrites, silence.MSG_UNSILENCE_SUCCESS), + ) + + for context, channel, role, overwrites, message in test_cases: + with self.subTest(ctx=context, channel=channel): + with mock.patch.object(self.cog, "send_message") as send_message: + with mock.patch.object(self.cog, "_force_voice_sync"): + await self.cog._unsilence_wrapper(channel, context) + + if context is None: + send_message.assert_called_once_with(message, channel, channel, True) + else: + send_message.assert_called_once_with(message, context.channel, channel, True) + + channel.set_permissions.assert_called_once_with(role, overwrite=overwrites) + if channel != ctx.channel: + ctx.channel.assert_not_called() + + await reset() -- cgit v1.2.3 From dc202302fa9bfa7ab54d80cc486acdd5b278ad6a Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 23:50:23 +0300 Subject: Finalizes Silence & Unsilence UnitTests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 49 ++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 1d05ee357..2392e6e59 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -415,9 +415,13 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): asyncio.run(self.cog._async_init()) # Populate instance attributes. - self.channel = MockTextChannel() - self.overwrite = PermissionOverwrite(stream=True, send_messages=True, add_reactions=False) - self.channel.overwrites_for.return_value = self.overwrite + self.text_channel = MockTextChannel() + self.text_overwrite = PermissionOverwrite(stream=True, send_messages=True, add_reactions=False) + self.text_channel.overwrites_for.return_value = self.text_overwrite + + self.voice_channel = MockVoiceChannel() + self.voice_overwrite = PermissionOverwrite(speak=True) + self.voice_channel.overwrites_for.return_value = self.voice_overwrite async def test_sent_correct_message(self): """Appropriate failure/success message was sent by the command.""" @@ -477,19 +481,19 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_silenced_channel(self): """Channel had `send_message` and `add_reactions` permissions revoked for verified role.""" - self.assertTrue(await self.cog._set_silence_overwrites(self.channel)) - self.assertFalse(self.overwrite.send_messages) - self.assertFalse(self.overwrite.add_reactions) - self.channel.set_permissions.assert_awaited_once_with( + self.assertTrue(await self.cog._set_silence_overwrites(self.text_channel)) + self.assertFalse(self.text_overwrite.send_messages) + self.assertFalse(self.text_overwrite.add_reactions) + self.text_channel.set_permissions.assert_awaited_once_with( self.cog._verified_msg_role, - overwrite=self.overwrite + overwrite=self.text_overwrite ) async def test_preserved_other_overwrites(self): """Channel's other unrelated overwrites were not changed.""" - prev_overwrite_dict = dict(self.overwrite) - await self.cog._set_silence_overwrites(self.channel) - new_overwrite_dict = dict(self.overwrite) + prev_overwrite_dict = dict(self.text_overwrite) + await self.cog._set_silence_overwrites(self.text_channel) + new_overwrite_dict = dict(self.text_overwrite) # Remove 'send_messages' & 'add_reactions' keys because they were changed by the method. del prev_overwrite_dict['send_messages'] @@ -520,8 +524,8 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_cached_previous_overwrites(self): """Channel's previous overwrites were cached.""" overwrite_json = '{"send_messages": true, "add_reactions": false}' - await self.cog._set_silence_overwrites(self.channel) - self.cog.previous_overwrites.set.assert_called_once_with(self.channel.id, overwrite_json) + await self.cog._set_silence_overwrites(self.text_channel) + self.cog.previous_overwrites.set.assert_called_once_with(self.text_channel.id, overwrite_json) @autospec(silence, "datetime") async def test_cached_unsilence_time(self, datetime_mock): @@ -531,7 +535,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): timestamp = now_timestamp + duration * 60 datetime_mock.now.return_value = datetime.fromtimestamp(now_timestamp, tz=timezone.utc) - ctx = MockContext(channel=self.channel) + ctx = MockContext(channel=self.text_channel) await self.cog.silence.callback(self.cog, ctx, duration) self.cog.unsilence_timestamps.set.assert_awaited_once_with(ctx.channel.id, timestamp) @@ -539,13 +543,13 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_cached_indefinite_time(self): """A value of -1 was cached for a permanent silence.""" - ctx = MockContext(channel=self.channel) + ctx = MockContext(channel=self.text_channel) await self.cog.silence.callback(self.cog, ctx, None) self.cog.unsilence_timestamps.set.assert_awaited_once_with(ctx.channel.id, -1) async def test_scheduled_task(self): """An unsilence task was scheduled.""" - ctx = MockContext(channel=self.channel, invoke=mock.MagicMock()) + ctx = MockContext(channel=self.text_channel, invoke=mock.MagicMock()) await self.cog.silence.callback(self.cog, ctx, 5) @@ -555,10 +559,21 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_permanent_not_scheduled(self): """A task was not scheduled for a permanent silence.""" - ctx = MockContext(channel=self.channel) + ctx = MockContext(channel=self.text_channel) await self.cog.silence.callback(self.cog, ctx, None) self.cog.scheduler.schedule_later.assert_not_called() + async def test_correct_permission_updates(self): + """Tests if _set_silence_overwrites can correctly get and update permissions.""" + self.assertTrue(await self.cog._set_silence_overwrites(self.text_channel)) + self.assertFalse(self.text_overwrite.send_messages or self.text_overwrite.add_reactions) + + self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel)) + self.assertFalse(self.voice_overwrite.speak) + + self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel, True)) + self.assertFalse(self.voice_overwrite.speak or self.voice_overwrite.connect) + @autospec(silence.Silence, "unsilence_timestamps", pass_mocks=False) class UnsilenceTests(unittest.IsolatedAsyncioTestCase): -- cgit v1.2.3 From 1cfd1a95a3d6351555b1520249b423ac7a9f2e06 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 11:52:45 +0300 Subject: Refractors For Style Guidelines Refractors method signatures and calls to follow python-discord style guide. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index c60ca9327..4d62588a1 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -117,9 +117,10 @@ class Silence(commands.Cog): if name in channel.name.lower(): return self.bot.get_channel(channels[name]) - async def send_message(self, message: str, source_channel: TextChannel, - target_channel: Union[TextChannel, VoiceChannel], - alert_target: bool = False, duration: HushDurationConverter = 0) -> None: + async def send_message( + self, message: str, source_channel: TextChannel, target_channel: Union[TextChannel, VoiceChannel], + alert_target: bool = False, duration: int = 0 + ) -> None: """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`.""" await source_channel.send( message.replace("current", target_channel.mention if source_channel != target_channel else "current") @@ -145,8 +146,10 @@ class Silence(commands.Cog): @commands.command(aliases=("hush",)) @lock_arg(LOCK_NAMESPACE, "ctx", attrgetter("channel"), raise_error=True) - async def silence(self, ctx: Context, duration: HushDurationConverter = 10, kick: bool = False, - *, channel: Optional[AnyChannelConverter] = None) -> None: + async def silence( + self, ctx: Context, duration: HushDurationConverter = 10, kick: bool = False, + *, channel: Optional[AnyChannelConverter] = None + ) -> None: """ Silence the current channel for `duration` minutes or `forever`. @@ -192,8 +195,9 @@ class Silence(commands.Cog): await self._unsilence_wrapper(channel, ctx) @lock_arg(LOCK_NAMESPACE, "channel", raise_error=True) - async def _unsilence_wrapper(self, channel: Union[TextChannel, VoiceChannel], - ctx: Optional[Context] = None) -> None: + async def _unsilence_wrapper( + self, channel: Union[TextChannel, VoiceChannel], ctx: Optional[Context] = None + ) -> None: """Unsilence `channel` and send a success/failure message.""" msg_channel = channel if ctx is not None: @@ -249,8 +253,9 @@ class Silence(commands.Cog): return True - async def _force_voice_sync(self, channel: VoiceChannel, member: Optional[Member] = None, - kick: bool = False) -> None: + async def _force_voice_sync( + self, channel: VoiceChannel, member: Optional[Member] = None, kick: bool = False + ) -> None: """ Move all non-staff members from `channel` to a temporary channel and back to force toggle role mute. @@ -283,8 +288,9 @@ class Silence(commands.Cog): log.info(f"Failed to get afk-channel, created temporary channel #{afk_channel} ({afk_channel.id})") # Schedule channel deletion in case function errors out - self.scheduler.schedule_later(30, afk_channel.id, - afk_channel.delete(reason="Deleting temp mute channel.")) + self.scheduler.schedule_later( + 30, afk_channel.id, afk_channel.delete(reason="Deleting temp mute channel.") + ) # Move all members to temporary channel and back for member in members: @@ -298,8 +304,9 @@ class Silence(commands.Cog): await member.move_to(channel, reason="Muting member.") log.debug(f"Moved {member.name} to original voice channel.") - async def _schedule_unsilence(self, ctx: Context, channel: Union[TextChannel, VoiceChannel], - duration: Optional[int]) -> None: + async def _schedule_unsilence( + self, ctx: Context, channel: Union[TextChannel, VoiceChannel], duration: Optional[int] + ) -> None: """Schedule `ctx.channel` to be unsilenced if `duration` is not None.""" if duration is None: await self.unsilence_timestamps.set(channel.id, -1) -- cgit v1.2.3 From b4e65a7b2578fe34296ed302c1ddba39df11bbd4 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 11:55:30 +0300 Subject: Fixes Voice Channel Access A typo caused the function to return the text channel for `code/help 1`, when it is meant to access `code/help 2`. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 4d62588a1..62f3ede73 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -109,7 +109,7 @@ class Silence(commands.Cog): channels = { "off-topic": Channels.voice_chat, "code/help 1": Channels.code_help_voice, - "code/help 2": Channels.code_help_voice, + "code/help 2": Channels.code_help_voice_2, "admin": Channels.admins_voice, "staff": Channels.staff_voice } -- cgit v1.2.3 From b2d88f860d5ca9d08c72be676a7bfdfd5943b417 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 12:20:01 +0300 Subject: Removes AnyChannel Converter Removes the AnyChannel converter in favor of a combination of Text and Voice converters. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/converters.py | 40 ----------------------------------- bot/exts/moderation/silence.py | 6 +++--- tests/bot/test_converters.py | 48 ------------------------------------------ 3 files changed, 3 insertions(+), 91 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index 613be73eb..2e118d476 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -536,46 +536,6 @@ class FetchedUser(UserConverter): raise BadArgument(f"User `{arg}` does not exist") -class AnyChannelConverter(Converter): - """ - Converts to a `discord.Channel` or, raises an error. - - Unlike the default Channel Converter, this converter can handle channels given - in string, id, or mention formatting for both `TextChannel`s and `VoiceChannel`s. - Always returns 1 or fewer channels, errors if more than one match exists. - - It is able to handle the following formats (caveats noted below:) - 1. Convert from ID - Example: 267631170882240512 - 2. Convert from Explicit Mention - Example: #welcome - 3. Convert from ID Mention - Example: <#267631170882240512> - 4. Convert from Unmentioned Name: - Example: welcome - - All the previous conversions are valid for both text and voice channels, but explicit - raw names (#4) do not work for non-unique channels, instead opting for an error. - Explicit mentions (#2) do not work for non-unique voice channels either. - """ - - async def convert(self, ctx: Context, arg: str) -> t.Union[discord.TextChannel, discord.VoiceChannel]: - """Convert the `arg` to a `TextChannel` or `VoiceChannel`.""" - stripped = arg.strip().lstrip("<").lstrip("#").rstrip(">").lower() - - # Filter channels by name and ID - channels = [channel for channel in ctx.guild.channels if stripped in (channel.name.lower(), str(channel.id))] - - if len(channels) == 0: - # Couldn't find a matching channel - log.debug(f"Could not convert `{arg}` to channel, no matches found.") - raise BadArgument(f"{arg} returned no matches.") - - elif len(channels) > 1: - # Couldn't discern the desired channel - log.debug(f"Could not convert `{arg}` to channel, {len(channels)} matches found.") - raise BadArgument(f"The provided argument returned too many matches ({len(channels)}).") - - else: - return channels[0] - - def _snowflake_from_regex(pattern: t.Pattern, arg: str) -> int: """ Extract the snowflake from `arg` using a regex `pattern` and return it as an int. diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 62f3ede73..8ad30f0d9 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -12,7 +12,7 @@ from discord.ext.commands import Context from bot.bot import Bot from bot.constants import Channels, Emojis, Guild, MODERATION_ROLES, Roles -from bot.converters import AnyChannelConverter, HushDurationConverter +from bot.converters import HushDurationConverter from bot.utils.lock import LockedResourceError, lock_arg from bot.utils.scheduling import Scheduler @@ -148,7 +148,7 @@ class Silence(commands.Cog): @lock_arg(LOCK_NAMESPACE, "ctx", attrgetter("channel"), raise_error=True) async def silence( self, ctx: Context, duration: HushDurationConverter = 10, kick: bool = False, - *, channel: Optional[AnyChannelConverter] = None + *, channel: Union[TextChannel, VoiceChannel] = None ) -> None: """ Silence the current channel for `duration` minutes or `forever`. @@ -182,7 +182,7 @@ class Silence(commands.Cog): await self.send_message(MSG_SILENCE_SUCCESS, ctx.channel, channel, True, duration) @commands.command(aliases=("unhush",)) - async def unsilence(self, ctx: Context, *, channel: AnyChannelConverter = None) -> None: + async def unsilence(self, ctx: Context, *, channel: Union[TextChannel, VoiceChannel] = None) -> None: """ Unsilence the given channel if given, else the current one. diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index 6bea71977..c42111f3f 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -7,7 +7,6 @@ from dateutil.relativedelta import relativedelta from discord.ext.commands import BadArgument from bot.converters import ( - AnyChannelConverter, Duration, HushDurationConverter, ISODateTime, @@ -25,18 +24,6 @@ class ConverterTests(unittest.IsolatedAsyncioTestCase): cls.context = MagicMock cls.context.author = 'bob' - cls.context.guild = MagicMock - cls.context.guild.channels = [] - - for i in range(10): - channel = MagicMock() - channel.name = f"test-{i}" - channel.id = str(i) * 18 - - cls.context.guild.channels.append(channel) - if i > 7: - cls.context.guild.channels.append(channel) - cls.fixed_utc_now = datetime.datetime.fromisoformat('2019-01-01T00:00:00') async def test_tag_content_converter_for_valid(self): @@ -325,38 +312,3 @@ class ConverterTests(unittest.IsolatedAsyncioTestCase): with self.subTest(invalid_minutes_string=invalid_minutes_string, exception_message=exception_message): with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): await converter.convert(self.context, invalid_minutes_string) - - async def test_any_channel_converter_for_valid(self): - """AnyChannelConverter returns correct channel from input.""" - test_values = ( - ("test-0", self.context.guild.channels[0]), - ("#test-0", self.context.guild.channels[0]), - (" #tEsT-0 ", self.context.guild.channels[0]), - (f"<#{'0' * 18}>", self.context.guild.channels[0]), - (f"{'0' * 18}", self.context.guild.channels[0]), - ("test-5", self.context.guild.channels[5]), - ("#test-5", self.context.guild.channels[5]), - (f"<#{'5' * 18}>", self.context.guild.channels[5]), - (f"{'5' * 18}", self.context.guild.channels[5]), - ) - - converter = AnyChannelConverter() - for input_string, expected_channel in test_values: - with self.subTest(input_string=input_string, expected_channel=expected_channel): - converted = await converter.convert(self.context, input_string) - self.assertEqual(expected_channel, converted) - - async def test_any_channel_converter_for_invalid(self): - """AnyChannelConverter raises BadArgument for invalid channels.""" - test_values = ( - ("#test-8", "The provided argument returned too many matches (2)."), - ("#test-9", "The provided argument returned too many matches (2)."), - ("#random-name", "#random-name returned no matches."), - ("test-10", "test-10 returned no matches.") - ) - - converter = AnyChannelConverter() - for invalid_input, exception_message in test_values: - with self.subTest(invalid_input=invalid_input, exception_message=exception_message): - with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): - await converter.convert(self.context, invalid_input) -- cgit v1.2.3 From c37e9842602f6e8e7bb2cc0897a6c8e0988f7cff Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 12:29:51 +0300 Subject: Fixes Typo in Doc Co-authored-by: Mark --- tests/bot/exts/moderation/test_silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 2392e6e59..5c6e2d0f1 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -564,7 +564,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.cog.scheduler.schedule_later.assert_not_called() async def test_correct_permission_updates(self): - """Tests if _set_silence_overwrites can correctly get and update permissions.""" + """Tests if _set_silence_overwrites can correctly get and update permissions.""" self.assertTrue(await self.cog._set_silence_overwrites(self.text_channel)) self.assertFalse(self.text_overwrite.send_messages or self.text_overwrite.add_reactions) -- cgit v1.2.3 From 7cb3024e71cd81e9ef29f5f10cb2bc5fe62ad846 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 13:19:51 +0300 Subject: Refractors SendMessage Function Refractors the send message function in silence to make it more understandable and flexible. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 31 ++++++++---------- tests/bot/exts/moderation/test_silence.py | 54 ++++++++++++++++++------------- 2 files changed, 46 insertions(+), 39 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 8ad30f0d9..7bc51ee93 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -26,7 +26,7 @@ MSG_SILENCE_SUCCESS = f"{Emojis.check_mark} silenced current channel for {{durat MSG_UNSILENCE_FAIL = f"{Emojis.cross_mark} current channel was not silenced." MSG_UNSILENCE_MANUAL = ( - f"{Emojis.cross_mark} current channel was not unsilenced because the channel overwrites were " + f"{Emojis.cross_mark} current channel was not unsilenced because the current overwrites were " f"set manually or the cache was prematurely cleared. " f"Please edit the overwrites manually to unsilence." ) @@ -119,30 +119,27 @@ class Silence(commands.Cog): async def send_message( self, message: str, source_channel: TextChannel, target_channel: Union[TextChannel, VoiceChannel], - alert_target: bool = False, duration: int = 0 + alert_target: bool = False ) -> None: """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`.""" - await source_channel.send( - message.replace("current", target_channel.mention if source_channel != target_channel else "current") - .replace("{duration}", str(duration)) - ) - + # Get TextChannel connected to VoiceChannel if channel is of type voice voice_chat = None if isinstance(target_channel, VoiceChannel): - # Send to relevant channel voice_chat = await self._get_related_text_channel(target_channel) - if alert_target and source_channel != target_channel: - if isinstance(target_channel, VoiceChannel): - if voice_chat is None or voice_chat == source_channel: - return + # Reply to invocation channel + source_reply = message + if source_channel != target_channel: + source_reply = source_reply.replace("current channel", target_channel.mention) + await source_channel.send(source_reply) - await voice_chat.send( - message.replace("{duration}", str(duration)).replace("current", target_channel.mention) - ) + # Reply to target channel + if alert_target and source_channel != target_channel and source_channel != voice_chat: + if isinstance(target_channel, VoiceChannel) and (voice_chat is not None or voice_chat != source_channel): + await voice_chat.send(message.replace("current channel", target_channel.mention)) else: - await target_channel.send(message.replace("{duration}", str(duration))) + await target_channel.send(message) @commands.command(aliases=("hush",)) @lock_arg(LOCK_NAMESPACE, "ctx", attrgetter("channel"), raise_error=True) @@ -179,7 +176,7 @@ class Silence(commands.Cog): else: log.info(f"Silenced {channel_info} for {duration} minute(s).") - await self.send_message(MSG_SILENCE_SUCCESS, ctx.channel, channel, True, duration) + await self.send_message(MSG_SILENCE_SUCCESS.format(duration=duration), ctx.channel, channel, True) @commands.command(aliases=("unhush",)) async def unsilence(self, ctx: Context, *, channel: Union[TextChannel, VoiceChannel] = None) -> None: diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 5c6e2d0f1..70678d207 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -7,7 +7,7 @@ from unittest.mock import Mock from async_rediscache import RedisSession from discord import PermissionOverwrite -from bot.constants import Channels, Emojis, Guild, Roles +from bot.constants import Channels, Guild, Roles from bot.exts.moderation import silence from tests.helpers import MockBot, MockContext, MockGuild, MockMember, MockTextChannel, MockVoiceChannel, autospec @@ -198,42 +198,48 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): reset() with self.subTest("Replacement One Channel Test"): - await self.cog.send_message("The following should be replaced: current", - text_channel_1, text_channel_2, False) - text_channel_1.send.assert_called_once_with(f"The following should be replaced: {text_channel_1.mention}") + await self.cog.send_message( + "Current. The following should be replaced: current channel.", text_channel_1, text_channel_2, False + ) + + text_channel_1.send.assert_called_once_with( + f"Current. The following should be replaced: {text_channel_1.mention}." + ) + text_channel_2.send.assert_not_called() reset() with self.subTest("Replacement Two Channel Test"): - await self.cog.send_message("The following should be replaced: current", - text_channel_1, text_channel_2, True) - text_channel_1.send.assert_called_once_with(f"The following should be replaced: {text_channel_1.mention}") - text_channel_2.send.assert_called_once_with("The following should be replaced: current") + await self.cog.send_message( + "Current. The following should be replaced: current channel.", text_channel_1, text_channel_2, True + ) - reset() - with self.subTest("Replace Duration"): - await self.cog.send_message(f"{Emojis.check_mark} The following should be replaced: {{duration}}", - text_channel_1, text_channel_2, False) - text_channel_1.send.assert_called_once_with(f"{Emojis.check_mark} The following should be replaced: 0") - text_channel_2.send.assert_not_called() + text_channel_1.send.assert_called_once_with( + f"Current. The following should be replaced: {text_channel_1.mention}." + ) + + text_channel_2.send.assert_called_once_with("Current. The following should be replaced: current channel.") reset() with self.subTest("Text and Voice"): - await self.cog.send_message("This should show up just here", - text_channel_1, voice_channel, False) + await self.cog.send_message( + "This should show up just here", text_channel_1, voice_channel, False + ) text_channel_1.send.assert_called_once_with("This should show up just here") reset() with self.subTest("Text and Voice"): - await self.cog.send_message("This should show up as current", - text_channel_1, voice_channel, True) + await self.cog.send_message( + "This should show up as current channel", text_channel_1, voice_channel, True + ) text_channel_1.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") reset() with self.subTest("Text and Voice Same Invocation"): - await self.cog.send_message("This should show up as current", - text_channel_2, voice_channel, True) + await self.cog.send_message( + "This should show up as current channel", text_channel_2, voice_channel, True + ) text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") async def test_get_related_text_channel(self): @@ -455,7 +461,9 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): if ctx.channel == target or target is None: ctx.channel.send.assert_called_once_with(message) else: - ctx.channel.send.assert_called_once_with(message.replace("current", text_channel.mention)) + ctx.channel.send.assert_called_once_with( + message.replace("current channel", text_channel.mention) + ) target.send.assert_called_once_with(message) ctx.channel.send.reset_mock() @@ -643,7 +651,9 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): if ctx.channel == target or target is None: ctx.channel.send.assert_called_once_with(message) else: - ctx.channel.send.assert_called_once_with(message.replace("current", text_channel.mention)) + ctx.channel.send.assert_called_once_with( + message.replace("current channel", text_channel.mention) + ) target.send.assert_called_once_with(message) ctx.channel.send.reset_mock() -- cgit v1.2.3 From d2d7db69c4e54120d36146e2334f702f4259ed6b Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 20:36:48 +0300 Subject: Moves VoiceChat Sync Out of Overwrites Function VoiceChat sync only needs to be called when the command is invoked, instead of while updating permissions. Moved call to command function to reflect that, and fixed failing tests. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 11 +++++++---- tests/bot/exts/moderation/test_silence.py | 12 +++++++----- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 7bc51ee93..64ffaa347 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -134,9 +134,10 @@ class Silence(commands.Cog): await source_channel.send(source_reply) # Reply to target channel - if alert_target and source_channel != target_channel and source_channel != voice_chat: - if isinstance(target_channel, VoiceChannel) and (voice_chat is not None or voice_chat != source_channel): - await voice_chat.send(message.replace("current channel", target_channel.mention)) + if alert_target and source_channel not in [target_channel, voice_chat]: + if isinstance(target_channel, VoiceChannel): + if voice_chat is not None: + await voice_chat.send(message.replace("current channel", target_channel.mention)) else: await target_channel.send(message) @@ -167,6 +168,9 @@ class Silence(commands.Cog): await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel) return + if isinstance(channel, VoiceChannel): + await self._force_voice_sync(channel, kick=kick) + await self._schedule_unsilence(ctx, channel, duration) if duration is None: @@ -244,7 +248,6 @@ class Silence(commands.Cog): overwrite.update(connect=False) await channel.set_permissions(self._verified_voice_role, overwrite=overwrite) - await self._force_voice_sync(channel, kick=kick) await self.previous_overwrites.set(channel.id, json.dumps(prev_overwrites)) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 70678d207..aab607392 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -446,11 +446,13 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_sent_to_correct_channel(self): """Test function sends messages to the correct channels.""" text_channel = MockTextChannel() + voice_channel = MockVoiceChannel() ctx = MockContext() test_cases = ( (None, silence.MSG_SILENCE_SUCCESS.format(duration=10)), (text_channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), + (voice_channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), (ctx.channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), ) @@ -460,14 +462,14 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) if ctx.channel == target or target is None: ctx.channel.send.assert_called_once_with(message) + else: - ctx.channel.send.assert_called_once_with( - message.replace("current channel", text_channel.mention) - ) - target.send.assert_called_once_with(message) + ctx.channel.send.assert_called_once_with(message.replace("current channel", target.mention)) + if isinstance(target, MockTextChannel): + target.send.assert_called_once_with(message) ctx.channel.send.reset_mock() - if target is not None: + if target is not None and isinstance(target, MockTextChannel): target.send.reset_mock() async def test_skipped_already_silenced(self): -- cgit v1.2.3 From e50bc5f507fe62afc534d286c5fc380d72c75b36 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 20:58:58 +0300 Subject: Move VoiceChat Sync To _unsilence Moves the call to voice chat sync from _unsilence_wrapper to _unsilence. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 64ffaa347..fd051a0ff 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -219,10 +219,6 @@ class Silence(commands.Cog): await self.send_message(MSG_UNSILENCE_FAIL, msg_channel, channel) else: - # Send success message to muted channel or voice chat channel, and invocation channel - if isinstance(channel, VoiceChannel): - await self._force_voice_sync(channel) - await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, True) async def _set_silence_overwrites(self, channel: Union[TextChannel, VoiceChannel], kick: bool = False) -> bool: @@ -345,6 +341,8 @@ class Silence(commands.Cog): await channel.set_permissions(self._verified_msg_role, overwrite=overwrite) else: await channel.set_permissions(self._verified_voice_role, overwrite=overwrite) + await self._force_voice_sync(channel) + log.info(f"Unsilenced channel #{channel} ({channel.id}).") self.scheduler.cancel(channel.id) -- cgit v1.2.3 From 675d7504977acafdb73d6a51e91228180a7c02a2 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 20:59:43 +0300 Subject: Fixes Typo in Silence Tests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index aab607392..8f3c1cb8b 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -821,6 +821,6 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): channel.set_permissions.assert_called_once_with(role, overwrite=overwrites) if channel != ctx.channel: - ctx.channel.assert_not_called() + ctx.channel.send.assert_not_called() await reset() -- cgit v1.2.3 From 7f44647e4cff507ffed200a4f29edb18775d4388 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 21:15:09 +0300 Subject: Adds Connect to Reset Permissions During unsilencing, if the previous channel overwrites are None, the channel should default to None for all relevant permissions. Adds the connect permission as it was missing. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index fd051a0ff..e13911d9e 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -333,7 +333,7 @@ class Silence(commands.Cog): if prev_overwrites is None: log.info(f"Missing previous overwrites for #{channel} ({channel.id}); defaulting to None.") - overwrite.update(send_messages=None, add_reactions=None, speak=None) + overwrite.update(send_messages=None, add_reactions=None, speak=None, connect=None) else: overwrite.update(**json.loads(prev_overwrites)) @@ -360,7 +360,7 @@ class Silence(commands.Cog): else: await self._mod_alerts_channel.send( f"<@&{Roles.admins}> Restored overwrites with default values after unsilencing " - f"{channel.mention}. Please check that the `Speak` " + f"{channel.mention}. Please check that the `Speak` and `Connect`" f"overwrites for {self._verified_voice_role.mention} are at their desired values." ) -- cgit v1.2.3 From 7456f17485481ac02538a7bc67d78845f56a0dd9 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 21:23:32 +0300 Subject: Reduces Redundancy in Unmute Replaces a repeated hardcoded message with a dynamically built one. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index e13911d9e..4196a2ac4 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -352,17 +352,17 @@ class Silence(commands.Cog): if prev_overwrites is None: if isinstance(channel, TextChannel): - await self._mod_alerts_channel.send( - f"<@&{Roles.admins}> Restored overwrites with default values after unsilencing " - f"{channel.mention}. Please check that the `Send Messages` and `Add Reactions` " - f"overwrites for {self._verified_msg_role.mention} are at their desired values." - ) + permissions = "`Send Messages` and `Add Reactions`" + role = self._verified_msg_role else: - await self._mod_alerts_channel.send( - f"<@&{Roles.admins}> Restored overwrites with default values after unsilencing " - f"{channel.mention}. Please check that the `Speak` and `Connect`" - f"overwrites for {self._verified_voice_role.mention} are at their desired values." - ) + permissions = "`Speak` and `Connect`" + role = self._verified_voice_role + + await self._mod_alerts_channel.send( + f"<@&{Roles.admins}> Restored overwrites with default values after unsilencing " + f"{channel.mention}. Please check that the {permissions} " + f"overwrites for {role.mention} are at their desired values." + ) return True -- cgit v1.2.3 From e4483ec353824167e573c4d86be5529ea329e97e Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 21:32:25 +0300 Subject: Reduces IsInstance Calls Where Possible Reduces redundant calls to isinstance by saving the result where applicable. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 4196a2ac4..69ad1f45e 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -124,7 +124,9 @@ class Silence(commands.Cog): """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`.""" # Get TextChannel connected to VoiceChannel if channel is of type voice voice_chat = None - if isinstance(target_channel, VoiceChannel): + target_is_voice_channel = isinstance(target_channel, VoiceChannel) + + if target_is_voice_channel: voice_chat = await self._get_related_text_channel(target_channel) # Reply to invocation channel @@ -135,7 +137,7 @@ class Silence(commands.Cog): # Reply to target channel if alert_target and source_channel not in [target_channel, voice_chat]: - if isinstance(target_channel, VoiceChannel): + if target_is_voice_channel: if voice_chat is not None: await voice_chat.send(message.replace("current channel", target_channel.mention)) @@ -223,7 +225,9 @@ class Silence(commands.Cog): async def _set_silence_overwrites(self, channel: Union[TextChannel, VoiceChannel], kick: bool = False) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" - if isinstance(channel, TextChannel): + is_text_channel = isinstance(channel, TextChannel) + + if is_text_channel: overwrite = channel.overwrites_for(self._verified_msg_role) prev_overwrites = dict(send_messages=overwrite.send_messages, add_reactions=overwrite.add_reactions) else: @@ -235,7 +239,7 @@ class Silence(commands.Cog): if channel.id in self.scheduler or all(val is False for val in prev_overwrites.values()): return False - if isinstance(channel, TextChannel): + if is_text_channel: overwrite.update(send_messages=False, add_reactions=False) await channel.set_permissions(self._verified_msg_role, overwrite=overwrite) else: @@ -326,7 +330,9 @@ class Silence(commands.Cog): log.info(f"Tried to unsilence channel #{channel} ({channel.id}) but the channel was not silenced.") return False - if isinstance(channel, TextChannel): + is_text_channel = isinstance(channel, TextChannel) + + if is_text_channel: overwrite = channel.overwrites_for(self._verified_msg_role) else: overwrite = channel.overwrites_for(self._verified_voice_role) @@ -337,7 +343,7 @@ class Silence(commands.Cog): else: overwrite.update(**json.loads(prev_overwrites)) - if isinstance(channel, TextChannel): + if is_text_channel: await channel.set_permissions(self._verified_msg_role, overwrite=overwrite) else: await channel.set_permissions(self._verified_voice_role, overwrite=overwrite) -- cgit v1.2.3 From 7c87400c05276534bbeb155a569d9c88ae83f0c6 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Wed, 25 Nov 2020 09:05:29 +0300 Subject: Refactors Send Message Function Refactors the send message utility function to make it more legible and reduce unnecessary calls. Co-authored-by: Mark --- bot/exts/moderation/silence.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 69ad1f45e..eaaf7e69b 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -123,12 +123,6 @@ class Silence(commands.Cog): ) -> None: """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`.""" # Get TextChannel connected to VoiceChannel if channel is of type voice - voice_chat = None - target_is_voice_channel = isinstance(target_channel, VoiceChannel) - - if target_is_voice_channel: - voice_chat = await self._get_related_text_channel(target_channel) - # Reply to invocation channel source_reply = message if source_channel != target_channel: @@ -136,12 +130,12 @@ class Silence(commands.Cog): await source_channel.send(source_reply) # Reply to target channel - if alert_target and source_channel not in [target_channel, voice_chat]: - if target_is_voice_channel: - if voice_chat is not None: + if alert_target: + if isinstance(target_channel, VoiceChannel): + voice_chat = await self._get_related_text_channel(target_channel) + if voice_chat and source_channel != voice_chat: await voice_chat.send(message.replace("current channel", target_channel.mention)) - - else: + elif source_channel != target_channel: await target_channel.send(message) @commands.command(aliases=("hush",)) -- cgit v1.2.3 From 03cd956165a806acd3f9e9b204129488142f12fd Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Wed, 25 Nov 2020 09:13:55 +0300 Subject: Reduces Redundant Code in Silence Cog Restructures some code to make it more understandable and reduce duplication. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 57 ++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index eaaf7e69b..f35214cf4 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -122,7 +122,6 @@ class Silence(commands.Cog): alert_target: bool = False ) -> None: """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`.""" - # Get TextChannel connected to VoiceChannel if channel is of type voice # Reply to invocation channel source_reply = message if source_channel != target_channel: @@ -219,30 +218,26 @@ class Silence(commands.Cog): async def _set_silence_overwrites(self, channel: Union[TextChannel, VoiceChannel], kick: bool = False) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" - is_text_channel = isinstance(channel, TextChannel) - - if is_text_channel: - overwrite = channel.overwrites_for(self._verified_msg_role) + # Get the original channel overwrites + if isinstance(channel, TextChannel): + role = self._verified_msg_role + overwrite = channel.overwrites_for(role) prev_overwrites = dict(send_messages=overwrite.send_messages, add_reactions=overwrite.add_reactions) + else: - overwrite = channel.overwrites_for(self._verified_voice_role) + role = self._verified_voice_role + overwrite = channel.overwrites_for(role) prev_overwrites = dict(speak=overwrite.speak) if kick: prev_overwrites.update(connect=overwrite.connect) + # Stop if channel was already silenced if channel.id in self.scheduler or all(val is False for val in prev_overwrites.values()): return False - if is_text_channel: - overwrite.update(send_messages=False, add_reactions=False) - await channel.set_permissions(self._verified_msg_role, overwrite=overwrite) - else: - overwrite.update(speak=False) - if kick: - overwrite.update(connect=False) - - await channel.set_permissions(self._verified_voice_role, overwrite=overwrite) - + # Set new permissions, store + overwrite.update(**dict.fromkeys(prev_overwrites, False)) + await channel.set_permissions(role, overwrite=overwrite) await self.previous_overwrites.set(channel.id, json.dumps(prev_overwrites)) return True @@ -319,28 +314,32 @@ class Silence(commands.Cog): Return `True` if channel permissions were changed, `False` otherwise. """ + # Get stored overwrites, and return if channel is unsilenced prev_overwrites = await self.previous_overwrites.get(channel.id) if channel.id not in self.scheduler and prev_overwrites is None: log.info(f"Tried to unsilence channel #{channel} ({channel.id}) but the channel was not silenced.") return False - is_text_channel = isinstance(channel, TextChannel) - - if is_text_channel: - overwrite = channel.overwrites_for(self._verified_msg_role) + # Select the role based on channel type, and get current overwrites + if isinstance(channel, TextChannel): + role = self._verified_msg_role + overwrite = channel.overwrites_for(role) + permissions = "`Send Messages` and `Add Reactions`" else: - overwrite = channel.overwrites_for(self._verified_voice_role) + role = self._verified_voice_role + overwrite = channel.overwrites_for(role) + permissions = "`Speak` and `Connect`" + # Check if old overwrites were not stored if prev_overwrites is None: log.info(f"Missing previous overwrites for #{channel} ({channel.id}); defaulting to None.") overwrite.update(send_messages=None, add_reactions=None, speak=None, connect=None) else: overwrite.update(**json.loads(prev_overwrites)) - if is_text_channel: - await channel.set_permissions(self._verified_msg_role, overwrite=overwrite) - else: - await channel.set_permissions(self._verified_voice_role, overwrite=overwrite) + # Update Permissions + await channel.set_permissions(role, overwrite=overwrite) + if isinstance(channel, VoiceChannel): await self._force_voice_sync(channel) log.info(f"Unsilenced channel #{channel} ({channel.id}).") @@ -350,14 +349,8 @@ class Silence(commands.Cog): await self.previous_overwrites.delete(channel.id) await self.unsilence_timestamps.delete(channel.id) + # Alert Admin team if old overwrites were not available if prev_overwrites is None: - if isinstance(channel, TextChannel): - permissions = "`Send Messages` and `Add Reactions`" - role = self._verified_msg_role - else: - permissions = "`Speak` and `Connect`" - role = self._verified_voice_role - await self._mod_alerts_channel.send( f"<@&{Roles.admins}> Restored overwrites with default values after unsilencing " f"{channel.mention}. Please check that the {permissions} " -- cgit v1.2.3 From e9ed70c02ea20871c41abe55649147a42b185c69 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Thu, 26 Nov 2020 22:17:09 +0300 Subject: Updates Silence Lock Modifies the lock on the silence command, in order to choose between ctx and channel arg based on input. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index f35214cf4..8c71d422d 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -2,7 +2,6 @@ import json import logging from contextlib import suppress from datetime import datetime, timedelta, timezone -from operator import attrgetter from typing import Optional, Union from async_rediscache import RedisCache @@ -13,7 +12,7 @@ from discord.ext.commands import Context from bot.bot import Bot from bot.constants import Channels, Emojis, Guild, MODERATION_ROLES, Roles from bot.converters import HushDurationConverter -from bot.utils.lock import LockedResourceError, lock_arg +from bot.utils.lock import LockedResourceError, lock, lock_arg from bot.utils.scheduling import Scheduler log = logging.getLogger(__name__) @@ -137,8 +136,17 @@ class Silence(commands.Cog): elif source_channel != target_channel: await target_channel.send(message) + async def _select_lock_channel(*args) -> Union[TextChannel, VoiceChannel]: + """Passes the channel to be silenced to the resource lock.""" + channel = args[0].get("channel") + if channel is not None: + return channel + + else: + return args[0].get("ctx").channel + @commands.command(aliases=("hush",)) - @lock_arg(LOCK_NAMESPACE, "ctx", attrgetter("channel"), raise_error=True) + @lock(LOCK_NAMESPACE, _select_lock_channel, raise_error=True) async def silence( self, ctx: Context, duration: HushDurationConverter = 10, kick: bool = False, *, channel: Union[TextChannel, VoiceChannel] = None -- cgit v1.2.3 From 3ea66f3a61720258e0dda44fc59e547692375280 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sat, 28 Nov 2020 20:13:54 +0300 Subject: Clarifies Constants Use in Silence Changes all usages of bot.constant to use dotted path to remove confusion and namespace collision. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 46 ++++++++++++++++--------------- tests/bot/exts/moderation/test_silence.py | 2 +- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 8c71d422d..d1db0da9b 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -5,12 +5,12 @@ from datetime import datetime, timedelta, timezone from typing import Optional, Union from async_rediscache import RedisCache -from discord import Member, PermissionOverwrite, TextChannel, VoiceChannel +from discord import Guild, PermissionOverwrite, TextChannel, VoiceChannel from discord.ext import commands, tasks from discord.ext.commands import Context +from bot import constants from bot.bot import Bot -from bot.constants import Channels, Emojis, Guild, MODERATION_ROLES, Roles from bot.converters import HushDurationConverter from bot.utils.lock import LockedResourceError, lock, lock_arg from bot.utils.scheduling import Scheduler @@ -19,17 +19,17 @@ log = logging.getLogger(__name__) LOCK_NAMESPACE = "silence" -MSG_SILENCE_FAIL = f"{Emojis.cross_mark} current channel is already silenced." -MSG_SILENCE_PERMANENT = f"{Emojis.check_mark} silenced current channel indefinitely." -MSG_SILENCE_SUCCESS = f"{Emojis.check_mark} silenced current channel for {{duration}} minute(s)." +MSG_SILENCE_FAIL = f"{constants.Emojis.cross_mark} current channel is already silenced." +MSG_SILENCE_PERMANENT = f"{constants.Emojis.check_mark} silenced current channel indefinitely." +MSG_SILENCE_SUCCESS = f"{constants.Emojis.check_mark} silenced current channel for {{duration}} minute(s)." -MSG_UNSILENCE_FAIL = f"{Emojis.cross_mark} current channel was not silenced." +MSG_UNSILENCE_FAIL = f"{constants.Emojis.cross_mark} current channel was not silenced." MSG_UNSILENCE_MANUAL = ( - f"{Emojis.cross_mark} current channel was not unsilenced because the current overwrites were " + f"{constants.Emojis.cross_mark} current channel was not unsilenced because the current overwrites were " f"set manually or the cache was prematurely cleared. " f"Please edit the overwrites manually to unsilence." ) -MSG_UNSILENCE_SUCCESS = f"{Emojis.check_mark} unsilenced current channel." +MSG_UNSILENCE_SUCCESS = f"{constants.Emojis.check_mark} unsilenced current channel." class SilenceNotifier(tasks.Loop): @@ -67,7 +67,9 @@ class SilenceNotifier(tasks.Loop): f"{channel.mention} for {(self._current_loop-start)//60} min" for channel, start in self._silenced_channels.items() ) - await self._alert_channel.send(f"<@&{Roles.moderators}> currently silenced channels: {channels_text}") + await self._alert_channel.send( + f"<@&{constants.Roles.moderators}> currently silenced channels: {channels_text}" + ) class Silence(commands.Cog): @@ -91,26 +93,26 @@ class Silence(commands.Cog): """Set instance attributes once the guild is available and reschedule unsilences.""" await self.bot.wait_until_guild_available() - guild = self.bot.get_guild(Guild.id) + guild = self.bot.get_guild(constants.Guild.id) - self._verified_msg_role = guild.get_role(Roles.verified) - self._verified_voice_role = guild.get_role(Roles.voice_verified) - self._helper_role = guild.get_role(Roles.helpers) + self._verified_msg_role = guild.get_role(constants.Roles.verified) + self._verified_voice_role = guild.get_role(constants.Roles.voice_verified) + self._helper_role = guild.get_role(constants.Roles.helpers) - self._mod_alerts_channel = self.bot.get_channel(Channels.mod_alerts) + self._mod_alerts_channel = self.bot.get_channel(constants.Channels.mod_alerts) - self.notifier = SilenceNotifier(self.bot.get_channel(Channels.mod_log)) + self.notifier = SilenceNotifier(self.bot.get_channel(constants.Channels.mod_log)) await self._reschedule() async def _get_related_text_channel(self, channel: VoiceChannel) -> Optional[TextChannel]: """Returns the text channel related to a voice channel.""" # TODO: Figure out a dynamic way of doing this channels = { - "off-topic": Channels.voice_chat, - "code/help 1": Channels.code_help_voice, - "code/help 2": Channels.code_help_voice_2, - "admin": Channels.admins_voice, - "staff": Channels.staff_voice + "off-topic": constants.Channels.voice_chat, + "code/help 1": constants.Channels.code_help_voice, + "code/help 2": constants.Channels.code_help_voice_2, + "admin": constants.Channels.admins_voice, + "staff": constants.Channels.staff_voice } for name in channels.keys(): if name in channel.name.lower(): @@ -360,7 +362,7 @@ class Silence(commands.Cog): # Alert Admin team if old overwrites were not available if prev_overwrites is None: await self._mod_alerts_channel.send( - f"<@&{Roles.admins}> Restored overwrites with default values after unsilencing " + f"<@&{constants.Roles.admins}> Restored overwrites with default values after unsilencing " f"{channel.mention}. Please check that the {permissions} " f"overwrites for {role.mention} are at their desired values." ) @@ -402,7 +404,7 @@ class Silence(commands.Cog): # This cannot be static (must have a __func__ attribute). async def cog_check(self, ctx: Context) -> bool: """Only allow moderators to invoke the commands in this cog.""" - return await commands.has_any_role(*MODERATION_ROLES).predicate(ctx) + return await commands.has_any_role(*constants.MODERATION_ROLES).predicate(ctx) def setup(bot: Bot) -> None: diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 8f3c1cb8b..bff2888b9 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -158,7 +158,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): self.assertTrue(self.cog._init_task.cancelled()) @autospec("discord.ext.commands", "has_any_role") - @mock.patch.object(silence, "MODERATION_ROLES", new=(1, 2, 3)) + @mock.patch.object(silence.constants, "MODERATION_ROLES", new=(1, 2, 3)) async def test_cog_check(self, role_check): """Role check was called with `MODERATION_ROLES`""" ctx = MockContext() -- cgit v1.2.3 From a6c8a9aca63f7d40d6c5701a08626da198c1d54a Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sat, 28 Nov 2020 20:14:48 +0300 Subject: Refractors Voice Sync Helper Refractors the voice sync helper function into two different functions, one for each purpose. Moves the afk_channel get/creation code to its own function. Updates tests. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 88 +++++++++++---------- tests/bot/exts/moderation/test_silence.py | 124 ++++++++++++++++++------------ 2 files changed, 121 insertions(+), 91 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index d1db0da9b..9b3725326 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -174,7 +174,10 @@ class Silence(commands.Cog): return if isinstance(channel, VoiceChannel): - await self._force_voice_sync(channel, kick=kick) + if kick: + await self._kick_voice_members(channel) + else: + await self._force_voice_sync(channel) await self._schedule_unsilence(ctx, channel, duration) @@ -252,56 +255,57 @@ class Silence(commands.Cog): return True - async def _force_voice_sync( - self, channel: VoiceChannel, member: Optional[Member] = None, kick: bool = False - ) -> None: - """ - Move all non-staff members from `channel` to a temporary channel and back to force toggle role mute. - - If `member` is passed, the mute only occurs to that member. - Permission modification has to happen before this function. - - If `kick_all` is True, members will not be added back to the voice channel. - """ - # Handle member picking logic - if member is not None: - members = [member] - else: - members = channel.members - - # Handle kick logic - if kick: - for member in members: - await member.move_to(None, reason="Kicking voice channel member.") + @staticmethod + async def _get_afk_channel(guild: Guild) -> VoiceChannel: + """Get a guild's AFK channel, or create one if it does not exist.""" + afk_channel = guild.afk_channel - log.debug(f"Kicked all members from #{channel.name} ({channel.id}).") - return - - # Obtain temporary channel - afk_channel = channel.guild.afk_channel if afk_channel is None: overwrites = { - channel.guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) + guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) } - afk_channel = await channel.guild.create_voice_channel("mute-temp", overwrites=overwrites) + afk_channel = await guild.create_voice_channel("mute-temp", overwrites=overwrites) log.info(f"Failed to get afk-channel, created temporary channel #{afk_channel} ({afk_channel.id})") - # Schedule channel deletion in case function errors out - self.scheduler.schedule_later( - 30, afk_channel.id, afk_channel.delete(reason="Deleting temp mute channel.") - ) + return afk_channel - # Move all members to temporary channel and back - for member in members: - # Skip staff - if self._helper_role in member.roles: - continue + async def _kick_voice_members(self, channel: VoiceChannel) -> None: + """Remove all non-staff members from a voice channel.""" + log.debug(f"Removing all non staff members from #{channel.name} ({channel.id}).") + + for member in channel.members: + if self._helper_role not in member.roles: + await member.move_to(None, reason="Kicking member from voice channel.") - await member.move_to(afk_channel, reason="Muting member.") - log.debug(f"Moved {member.name} to afk channel.") + log.debug("Removed all members.") - await member.move_to(channel, reason="Muting member.") - log.debug(f"Moved {member.name} to original voice channel.") + async def _force_voice_sync(self, channel: VoiceChannel) -> None: + """ + Move all non-staff members from `channel` to a temporary channel and back to force toggle role mute. + + Permission modification has to happen before this function. + """ + # Obtain temporary channel + delete_channel = channel.guild.afk_channel is None + afk_channel = await self._get_afk_channel(channel.guild) + + try: + # Move all members to temporary channel and back + for member in channel.members: + # Skip staff + if self._helper_role in member.roles: + continue + + await member.move_to(afk_channel, reason="Muting VC member.") + log.debug(f"Moved {member.name} to afk channel.") + + await member.move_to(channel, reason="Muting VC member.") + log.debug(f"Moved {member.name} to original voice channel.") + + finally: + # Delete VC channel if it was created. + if delete_channel: + await afk_channel.delete(reason="Deleting temp mute channel.") async def _schedule_unsilence( self, ctx: Context, channel: Union[TextChannel, VoiceChannel], duration: Optional[int] diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index bff2888b9..9fb3e404a 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -2,7 +2,7 @@ import asyncio import unittest from datetime import datetime, timezone from unittest import mock -from unittest.mock import Mock +from unittest.mock import AsyncMock, Mock from async_rediscache import RedisSession from discord import PermissionOverwrite @@ -266,67 +266,69 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): """Tests the _force_voice_sync helper function.""" await self.cog._async_init() - afk_channel = MockVoiceChannel() - channel = MockVoiceChannel(guild=MockGuild(afk_channel=afk_channel)) - members = [] for _ in range(10): members.append(MockMember()) - channel.members = members - test_cases = ( - (members[0], False, "Muting member."), - (members[0], True, "Kicking voice channel member."), - (None, False, "Muting member."), - (None, True, "Kicking voice channel member."), - ) - - for member, kick, reason in test_cases: - with self.subTest(members=member, kick=kick, reason=reason): - await self.cog._force_voice_sync(channel, member, kick) - - for single_member in channel.members if member is None else [member]: - if kick: - single_member.move_to.assert_called_once_with(None, reason=reason) - else: - self.assertEqual(single_member.move_to.call_count, 2) - single_member.move_to.assert_has_calls([ - mock.call(afk_channel, reason=reason), - mock.call(channel, reason=reason) - ], any_order=False) + afk_channel = MockVoiceChannel() + channel = MockVoiceChannel(guild=MockGuild(afk_channel=afk_channel), members=members) - single_member.reset_mock() + await self.cog._force_voice_sync(channel) + for member in members: + self.assertEqual(member.move_to.call_count, 2) + member.move_to.assert_has_calls([ + mock.call(afk_channel, reason="Muting VC member."), + mock.call(channel, reason="Muting VC member.") + ], any_order=False) async def test_force_voice_sync_staff(self): """Tests to ensure _force_voice_sync does not kick staff members.""" await self.cog._async_init() member = MockMember(roles=[self.cog._helper_role]) - await self.cog._force_voice_sync(MockVoiceChannel(), member) + await self.cog._force_voice_sync(MockVoiceChannel(members=[member])) member.move_to.assert_not_called() async def test_force_voice_sync_no_channel(self): """Test to ensure _force_voice_sync can create its own voice channel if one is not available.""" await self.cog._async_init() - member = MockMember() channel = MockVoiceChannel(guild=MockGuild(afk_channel=None)) - - new_channel = MockVoiceChannel(delete=Mock()) + new_channel = MockVoiceChannel(delete=AsyncMock()) channel.guild.create_voice_channel.return_value = new_channel - with mock.patch.object(self.cog.scheduler, "schedule_later") as scheduler: - await self.cog._force_voice_sync(channel, member) + await self.cog._force_voice_sync(channel) + + # Check channel creation + overwrites = { + channel.guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) + } + channel.guild.create_voice_channel.assert_called_once_with("mute-temp", overwrites=overwrites) + + # Check bot deleted channel + new_channel.delete.assert_called_once_with(reason="Deleting temp mute channel.") + + async def test_voice_kick(self): + """Test to ensure kick function can remove all members from a voice channel.""" + await self.cog._async_init() + + members = [] + for _ in range(10): + members.append(MockMember()) + + channel = MockVoiceChannel(members=members) + await self.cog._kick_voice_members(channel) + + for member in members: + member.move_to.assert_called_once_with(None, reason="Kicking member from voice channel.") - # Check channel creation - overwrites = { - channel.guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) - } - channel.guild.create_voice_channel.assert_called_once_with("mute-temp", overwrites=overwrites) + async def test_voice_kick_staff(self): + """Test to ensure voice kick skips staff members.""" + await self.cog._async_init() + member = MockMember(roles=[self.cog._helper_role]) - # Check bot queued deletion - new_channel.delete.assert_called_once_with(reason="Deleting temp mute channel.") - scheduler.assert_called_once_with(30, new_channel.id, new_channel.delete()) + await self.cog._kick_voice_members(MockVoiceChannel(members=[member])) + member.move_to.assert_not_called() @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) @@ -457,21 +459,45 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): ) for target, message in test_cases: - with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): - with self.subTest(target_channel=target, message=message): - await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) - if ctx.channel == target or target is None: - ctx.channel.send.assert_called_once_with(message) + with mock.patch.object(self.cog, "_force_voice_sync") as voice_sync: + with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): + with self.subTest(target_channel=target, message=message): + await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) + if ctx.channel == target or target is None: + ctx.channel.send.assert_called_once_with(message) - else: - ctx.channel.send.assert_called_once_with(message.replace("current channel", target.mention)) - if isinstance(target, MockTextChannel): - target.send.assert_called_once_with(message) + else: + ctx.channel.send.assert_called_once_with(message.replace("current channel", target.mention)) + if isinstance(target, MockTextChannel): + target.send.assert_called_once_with(message) + else: + voice_sync.assert_called_once_with(target) ctx.channel.send.reset_mock() if target is not None and isinstance(target, MockTextChannel): target.send.reset_mock() + @mock.patch.object(silence.Silence, "_kick_voice_members") + @mock.patch.object(silence.Silence, "_force_voice_sync") + async def test_sync_or_kick_called(self, sync, kick): + """Tests if silence command calls kick or sync on voice channels when appropriate.""" + channel = MockVoiceChannel() + ctx = MockContext() + + with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): + with self.subTest("Test calls kick"): + await self.cog.silence.callback(self.cog, ctx, 10, kick=True, channel=channel) + kick.assert_called_once_with(channel) + sync.assert_not_called() + + kick.reset_mock() + sync.reset_mock() + + with self.subTest("Test calls sync"): + await self.cog.silence.callback(self.cog, ctx, 10, kick=False, channel=channel) + sync.assert_called_once_with(channel) + kick.assert_not_called() + async def test_skipped_already_silenced(self): """Permissions were not set and `False` was returned for an already silenced channel.""" subtests = ( -- cgit v1.2.3 From a8a8c104823fa1a23a9b33cd52c7c4e574d84330 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sat, 28 Nov 2020 21:10:24 +0300 Subject: Refractors According To Style Guide Updates changes made in the PR to be more inline with style guide. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 32 ++++---- tests/bot/exts/moderation/test_silence.py | 120 ++++++++++++++---------------- 2 files changed, 74 insertions(+), 78 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 9b3725326..45c3f5b92 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -31,6 +31,8 @@ MSG_UNSILENCE_MANUAL = ( ) MSG_UNSILENCE_SUCCESS = f"{constants.Emojis.check_mark} unsilenced current channel." +TextOrVoiceChannel = Union[TextChannel, VoiceChannel] + class SilenceNotifier(tasks.Loop): """Loop notifier for posting notices to `alert_channel` containing added channels.""" @@ -40,7 +42,7 @@ class SilenceNotifier(tasks.Loop): self._silenced_channels = {} self._alert_channel = alert_channel - def add_channel(self, channel: Union[TextChannel, VoiceChannel]) -> None: + def add_channel(self, channel: TextOrVoiceChannel) -> None: """Add channel to `_silenced_channels` and start loop if not launched.""" if not self._silenced_channels: self.start() @@ -119,7 +121,10 @@ class Silence(commands.Cog): return self.bot.get_channel(channels[name]) async def send_message( - self, message: str, source_channel: TextChannel, target_channel: Union[TextChannel, VoiceChannel], + self, + message: str, + source_channel: TextChannel, + target_channel: TextOrVoiceChannel, alert_target: bool = False ) -> None: """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`.""" @@ -138,7 +143,7 @@ class Silence(commands.Cog): elif source_channel != target_channel: await target_channel.send(message) - async def _select_lock_channel(*args) -> Union[TextChannel, VoiceChannel]: + async def _select_lock_channel(*args) -> TextOrVoiceChannel: """Passes the channel to be silenced to the resource lock.""" channel = args[0].get("channel") if channel is not None: @@ -150,8 +155,11 @@ class Silence(commands.Cog): @commands.command(aliases=("hush",)) @lock(LOCK_NAMESPACE, _select_lock_channel, raise_error=True) async def silence( - self, ctx: Context, duration: HushDurationConverter = 10, kick: bool = False, - *, channel: Union[TextChannel, VoiceChannel] = None + self, + ctx: Context, + duration: HushDurationConverter = 10, + kick: bool = False, + *, channel: TextOrVoiceChannel = None ) -> None: """ Silence the current channel for `duration` minutes or `forever`. @@ -191,7 +199,7 @@ class Silence(commands.Cog): await self.send_message(MSG_SILENCE_SUCCESS.format(duration=duration), ctx.channel, channel, True) @commands.command(aliases=("unhush",)) - async def unsilence(self, ctx: Context, *, channel: Union[TextChannel, VoiceChannel] = None) -> None: + async def unsilence(self, ctx: Context, *, channel: TextOrVoiceChannel = None) -> None: """ Unsilence the given channel if given, else the current one. @@ -204,9 +212,7 @@ class Silence(commands.Cog): await self._unsilence_wrapper(channel, ctx) @lock_arg(LOCK_NAMESPACE, "channel", raise_error=True) - async def _unsilence_wrapper( - self, channel: Union[TextChannel, VoiceChannel], ctx: Optional[Context] = None - ) -> None: + async def _unsilence_wrapper(self, channel: TextOrVoiceChannel, ctx: Optional[Context] = None) -> None: """Unsilence `channel` and send a success/failure message.""" msg_channel = channel if ctx is not None: @@ -229,7 +235,7 @@ class Silence(commands.Cog): else: await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, True) - async def _set_silence_overwrites(self, channel: Union[TextChannel, VoiceChannel], kick: bool = False) -> bool: + async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, kick: bool = False) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" # Get the original channel overwrites if isinstance(channel, TextChannel): @@ -307,9 +313,7 @@ class Silence(commands.Cog): if delete_channel: await afk_channel.delete(reason="Deleting temp mute channel.") - async def _schedule_unsilence( - self, ctx: Context, channel: Union[TextChannel, VoiceChannel], duration: Optional[int] - ) -> None: + async def _schedule_unsilence(self, ctx: Context, channel: TextOrVoiceChannel, duration: Optional[int]) -> None: """Schedule `ctx.channel` to be unsilenced if `duration` is not None.""" if duration is None: await self.unsilence_timestamps.set(channel.id, -1) @@ -318,7 +322,7 @@ class Silence(commands.Cog): unsilence_time = datetime.now(tz=timezone.utc) + timedelta(minutes=duration) await self.unsilence_timestamps.set(channel.id, unsilence_time.timestamp()) - async def _unsilence(self, channel: Union[TextChannel, VoiceChannel]) -> bool: + async def _unsilence(self, channel: TextOrVoiceChannel) -> bool: """ Unsilence `channel`. diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 9fb3e404a..635e017e3 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -198,49 +198,37 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): reset() with self.subTest("Replacement One Channel Test"): - await self.cog.send_message( - "Current. The following should be replaced: current channel.", text_channel_1, text_channel_2, False - ) - - text_channel_1.send.assert_called_once_with( - f"Current. The following should be replaced: {text_channel_1.mention}." - ) + message = "Current. The following should be replaced: current channel." + await self.cog.send_message(message, text_channel_1, text_channel_2, False) + text_channel_1.send.assert_called_once_with(message.replace("current channel", text_channel_1.mention)) text_channel_2.send.assert_not_called() reset() with self.subTest("Replacement Two Channel Test"): - await self.cog.send_message( - "Current. The following should be replaced: current channel.", text_channel_1, text_channel_2, True - ) - - text_channel_1.send.assert_called_once_with( - f"Current. The following should be replaced: {text_channel_1.mention}." - ) + message = "Current. The following should be replaced: current channel." + await self.cog.send_message(message, text_channel_1, text_channel_2, True) - text_channel_2.send.assert_called_once_with("Current. The following should be replaced: current channel.") + text_channel_1.send.assert_called_once_with(message.replace("current channel", text_channel_1.mention)) + text_channel_2.send.assert_called_once_with(message) reset() with self.subTest("Text and Voice"): - await self.cog.send_message( - "This should show up just here", text_channel_1, voice_channel, False - ) + await self.cog.send_message("This should show up just here", text_channel_1, voice_channel, False) text_channel_1.send.assert_called_once_with("This should show up just here") reset() with self.subTest("Text and Voice"): - await self.cog.send_message( - "This should show up as current channel", text_channel_1, voice_channel, True - ) - text_channel_1.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") - text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") + message = "This should show up as current channel" + await self.cog.send_message(message, text_channel_1, voice_channel, True) + text_channel_1.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) + text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) reset() with self.subTest("Text and Voice Same Invocation"): - await self.cog.send_message( - "This should show up as current channel", text_channel_2, voice_channel, True - ) - text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") + message = "This should show up as current channel" + await self.cog.send_message(message, text_channel_2, voice_channel, True) + text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) async def test_get_related_text_channel(self): """Tests the helper function that connects voice to text channels.""" @@ -276,10 +264,11 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): await self.cog._force_voice_sync(channel) for member in members: self.assertEqual(member.move_to.call_count, 2) - member.move_to.assert_has_calls([ + calls = [ mock.call(afk_channel, reason="Muting VC member."), mock.call(channel, reason="Muting VC member.") - ], any_order=False) + ] + member.move_to.assert_has_calls(calls, any_order=False) async def test_force_voice_sync_staff(self): """Tests to ensure _force_voice_sync does not kick staff members.""" @@ -445,7 +434,9 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.silence.callback(self.cog, ctx, duration) ctx.channel.send.assert_called_once_with(message) - async def test_sent_to_correct_channel(self): + @mock.patch.object(silence.Silence, "_set_silence_overwrites", return_value=True) + @mock.patch.object(silence.Silence, "_force_voice_sync") + async def test_sent_to_correct_channel(self, voice_sync, _): """Test function sends messages to the correct channels.""" text_channel = MockTextChannel() voice_channel = MockVoiceChannel() @@ -459,19 +450,17 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): ) for target, message in test_cases: - with mock.patch.object(self.cog, "_force_voice_sync") as voice_sync: - with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): - with self.subTest(target_channel=target, message=message): - await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) - if ctx.channel == target or target is None: - ctx.channel.send.assert_called_once_with(message) - - else: - ctx.channel.send.assert_called_once_with(message.replace("current channel", target.mention)) - if isinstance(target, MockTextChannel): - target.send.assert_called_once_with(message) - else: - voice_sync.assert_called_once_with(target) + with self.subTest(target_channel=target, message=message): + await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) + if ctx.channel == target or target is None: + ctx.channel.send.assert_called_once_with(message) + + else: + ctx.channel.send.assert_called_once_with(message.replace("current channel", target.mention)) + if isinstance(target, MockTextChannel): + target.send.assert_called_once_with(message) + else: + voice_sync.assert_called_once_with(target) ctx.channel.send.reset_mock() if target is not None and isinstance(target, MockTextChannel): @@ -771,7 +760,9 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) - async def test_unsilence_helper_fail(self): + @mock.patch.object(silence.Silence, "_unsilence", return_value=False) + @mock.patch.object(silence.Silence, "send_message") + async def test_unsilence_helper_fail(self, send_message, _): """Tests unsilence_wrapper when `_unsilence` fails.""" ctx = MockContext() @@ -797,16 +788,18 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): return self.val for context, channel, role, permission, message in test_cases: - with self.subTest(channel=channel, message=message): - with mock.patch.object(channel, "overwrites_for", return_value=PermClass(permission)) as overwrites: - with mock.patch.object(self.cog, "send_message") as send_message: - with mock.patch.object(self.cog, "_unsilence", return_value=False): - await self.cog._unsilence_wrapper(channel, context) + with mock.patch.object(channel, "overwrites_for", return_value=PermClass(permission)) as overwrites: + with self.subTest(channel=channel, message=message): + await self.cog._unsilence_wrapper(channel, context) + + overwrites.assert_called_once_with(role) + send_message.assert_called_once_with(message, ctx.channel, channel) - overwrites.assert_called_once_with(role) - send_message.assert_called_once_with(message, ctx.channel, channel) + send_message.reset_mock() - async def test_correct_overwrites(self): + @mock.patch.object(silence.Silence, "_force_voice_sync") + @mock.patch.object(silence.Silence, "send_message") + async def test_correct_overwrites(self, send_message, _): """Tests the overwrites returned by the _unsilence_wrapper are correct for voice and text channels.""" ctx = MockContext() @@ -822,6 +815,7 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): text_channel.reset_mock() voice_channel.reset_mock() + send_message.reset_mock() await reset() default_text_overwrites = text_channel.overwrites_for(text_role) @@ -836,17 +830,15 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): for context, channel, role, overwrites, message in test_cases: with self.subTest(ctx=context, channel=channel): - with mock.patch.object(self.cog, "send_message") as send_message: - with mock.patch.object(self.cog, "_force_voice_sync"): - await self.cog._unsilence_wrapper(channel, context) - - if context is None: - send_message.assert_called_once_with(message, channel, channel, True) - else: - send_message.assert_called_once_with(message, context.channel, channel, True) - - channel.set_permissions.assert_called_once_with(role, overwrite=overwrites) - if channel != ctx.channel: - ctx.channel.send.assert_not_called() + await self.cog._unsilence_wrapper(channel, context) + + if context is None: + send_message.assert_called_once_with(message, channel, channel, True) + else: + send_message.assert_called_once_with(message, context.channel, channel, True) + + channel.set_permissions.assert_called_once_with(role, overwrite=overwrites) + if channel != ctx.channel: + ctx.channel.send.assert_not_called() await reset() -- cgit v1.2.3 From 301978174cac156282b89fe6e749edc611824b8d Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 1 Dec 2020 10:17:55 +0300 Subject: Improves Voice Chat Matching Changes the way voice channels are matched with chat channels, to make it less hardcoded. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 45c3f5b92..4cc89827f 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -33,6 +33,13 @@ MSG_UNSILENCE_SUCCESS = f"{constants.Emojis.check_mark} unsilenced current chann TextOrVoiceChannel = Union[TextChannel, VoiceChannel] +VOICE_CHANNELS = { + constants.Channels.code_help_voice_1: constants.Channels.code_help_chat_1, + constants.Channels.code_help_voice_2: constants.Channels.code_help_chat_2, + constants.Channels.general_voice: constants.Channels.voice_chat, + constants.Channels.staff_voice: constants.Channels.staff_voice_chat, +} + class SilenceNotifier(tasks.Loop): """Loop notifier for posting notices to `alert_channel` containing added channels.""" @@ -106,20 +113,6 @@ class Silence(commands.Cog): self.notifier = SilenceNotifier(self.bot.get_channel(constants.Channels.mod_log)) await self._reschedule() - async def _get_related_text_channel(self, channel: VoiceChannel) -> Optional[TextChannel]: - """Returns the text channel related to a voice channel.""" - # TODO: Figure out a dynamic way of doing this - channels = { - "off-topic": constants.Channels.voice_chat, - "code/help 1": constants.Channels.code_help_voice, - "code/help 2": constants.Channels.code_help_voice_2, - "admin": constants.Channels.admins_voice, - "staff": constants.Channels.staff_voice - } - for name in channels.keys(): - if name in channel.name.lower(): - return self.bot.get_channel(channels[name]) - async def send_message( self, message: str, @@ -137,9 +130,10 @@ class Silence(commands.Cog): # Reply to target channel if alert_target: if isinstance(target_channel, VoiceChannel): - voice_chat = await self._get_related_text_channel(target_channel) + voice_chat = self.bot.get_channel(VOICE_CHANNELS.get(target_channel.id)) if voice_chat and source_channel != voice_chat: await voice_chat.send(message.replace("current channel", target_channel.mention)) + elif source_channel != target_channel: await target_channel.send(message) -- cgit v1.2.3 From ca266629c8083c261208ddba86ffe9e2a8b65bf3 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 1 Dec 2020 10:19:22 +0300 Subject: Fixes Voice Silence Tests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 66 +++++++++++++++---------------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 635e017e3..90ddf6ad7 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -168,8 +168,8 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): role_check.assert_called_once_with(*(1, 2, 3)) role_check.return_value.predicate.assert_awaited_once_with(ctx) - @mock.patch.object(silence.Silence, "_get_related_text_channel") - async def test_send_message(self, mock_get_related_text_channel): + @mock.patch.object(silence, "VOICE_CHANNELS") + async def test_send_message(self, mock_voice_channels): """Test the send function reports to the correct channels.""" text_channel_1 = MockTextChannel() text_channel_2 = MockTextChannel() @@ -178,12 +178,13 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): voice_channel.name = "General/Offtopic" voice_channel.mention = f"#{voice_channel.name}" - mock_get_related_text_channel.return_value = text_channel_2 + mock_voice_channels.get.return_value = text_channel_2.id def reset(): text_channel_1.reset_mock() text_channel_2.reset_mock() voice_channel.reset_mock() + mock_voice_channels.reset_mock() with self.subTest("Basic One Channel Test"): await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, False) @@ -217,38 +218,29 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): await self.cog.send_message("This should show up just here", text_channel_1, voice_channel, False) text_channel_1.send.assert_called_once_with("This should show up just here") - reset() - with self.subTest("Text and Voice"): - message = "This should show up as current channel" - await self.cog.send_message(message, text_channel_1, voice_channel, True) - text_channel_1.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) + with mock.patch.object(self.cog, "bot") as bot_mock: + bot_mock.get_channel.return_value = text_channel_2 - reset() - with self.subTest("Text and Voice Same Invocation"): - message = "This should show up as current channel" - await self.cog.send_message(message, text_channel_2, voice_channel, True) - text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - - async def test_get_related_text_channel(self): - """Tests the helper function that connects voice to text channels.""" - voice_channel = MockVoiceChannel() + reset() + with self.subTest("Text and Voice"): + message = "This should show up as current channel" + await self.cog.send_message(message, text_channel_1, voice_channel, True) + text_channel_1.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) + text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - tests = ( - ("Off-Topic/General", Channels.voice_chat), - ("code/help 1", Channels.code_help_voice), - ("Staff", Channels.staff_voice), - ("ADMIN", Channels.admins_voice), - ("not in the channel list", None) - ) + mock_voice_channels.get.assert_called_once_with(voice_channel.id) + bot_mock.get_channel.assert_called_once_with(text_channel_2.id) + bot_mock.reset_mock() - with mock.patch.object(self.cog.bot, "get_channel", lambda x: x): - for (name, channel_id) in tests: - voice_channel.name = name - voice_channel.id = channel_id + reset() + with self.subTest("Text and Voice Same Invocation"): + message = "This should show up as current channel" + await self.cog.send_message(message, text_channel_2, voice_channel, True) + text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - result_id = await self.cog._get_related_text_channel(voice_channel) - self.assertEqual(result_id, channel_id) + mock_voice_channels.get.assert_called_once_with(voice_channel.id) + bot_mock.get_channel.assert_called_once_with(text_channel_2.id) + bot_mock.reset_mock() async def test_force_voice_sync(self): """Tests the _force_voice_sync helper function.""" @@ -451,7 +443,10 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): for target, message in test_cases: with self.subTest(target_channel=target, message=message): - await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) + with mock.patch.object(self.cog, "bot") as bot_mock: + bot_mock.get_channel.return_value = AsyncMock() + await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) + if ctx.channel == target or target is None: ctx.channel.send.assert_called_once_with(message) @@ -466,14 +461,17 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): if target is not None and isinstance(target, MockTextChannel): target.send.reset_mock() + @mock.patch.object(silence.Silence, "_set_silence_overwrites", return_value=True) @mock.patch.object(silence.Silence, "_kick_voice_members") @mock.patch.object(silence.Silence, "_force_voice_sync") - async def test_sync_or_kick_called(self, sync, kick): + async def test_sync_or_kick_called(self, sync, kick, _): """Tests if silence command calls kick or sync on voice channels when appropriate.""" channel = MockVoiceChannel() ctx = MockContext() - with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): + with mock.patch.object(self.cog, "bot") as bot_mock: + bot_mock.get_channel.return_value = AsyncMock() + with self.subTest("Test calls kick"): await self.cog.silence.callback(self.cog, ctx, 10, kick=True, channel=channel) kick.assert_called_once_with(channel) -- cgit v1.2.3 From 68c2545e794fa284a957809a0cb1022740966118 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 29 Dec 2020 19:11:51 +0300 Subject: Refractors Helper Method Signatures Changes the signatures of a few helper methods to make them more concise and understandable. --- bot/exts/moderation/silence.py | 34 ++++++++++++++++--------------- tests/bot/exts/moderation/test_silence.py | 23 +++++++++++---------- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index befcb2cc4..31103bc3e 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -2,7 +2,7 @@ import json import logging from contextlib import suppress from datetime import datetime, timedelta, timezone -from typing import Optional, Union +from typing import Optional, OrderedDict, Union from async_rediscache import RedisCache from discord import Guild, PermissionOverwrite, TextChannel, VoiceChannel @@ -81,6 +81,16 @@ class SilenceNotifier(tasks.Loop): ) +async def _select_lock_channel(args: OrderedDict[str, any]) -> TextOrVoiceChannel: + """Passes the channel to be silenced to the resource lock.""" + channel = args["channel"] + if channel is not None: + return channel + + else: + return args["ctx"].channel + + class Silence(commands.Cog): """Commands for stopping channel messages for `verified` role in a channel.""" @@ -118,7 +128,7 @@ class Silence(commands.Cog): message: str, source_channel: TextChannel, target_channel: TextOrVoiceChannel, - alert_target: bool = False + *, alert_target: bool = False ) -> None: """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`.""" # Reply to invocation channel @@ -137,23 +147,14 @@ class Silence(commands.Cog): elif source_channel != target_channel: await target_channel.send(message) - async def _select_lock_channel(*args) -> TextOrVoiceChannel: - """Passes the channel to be silenced to the resource lock.""" - channel = args[0].get("channel") - if channel is not None: - return channel - - else: - return args[0].get("ctx").channel - @commands.command(aliases=("hush",)) @lock(LOCK_NAMESPACE, _select_lock_channel, raise_error=True) async def silence( self, ctx: Context, duration: HushDurationConverter = 10, - kick: bool = False, - *, channel: TextOrVoiceChannel = None + channel: TextOrVoiceChannel = None, + *, kick: bool = False ) -> None: """ Silence the current channel for `duration` minutes or `forever`. @@ -186,11 +187,12 @@ class Silence(commands.Cog): if duration is None: self.notifier.add_channel(channel) log.info(f"Silenced {channel_info} indefinitely.") - await self.send_message(MSG_SILENCE_PERMANENT, ctx.channel, channel, True) + await self.send_message(MSG_SILENCE_PERMANENT, ctx.channel, channel, alert_target=True) else: log.info(f"Silenced {channel_info} for {duration} minute(s).") - await self.send_message(MSG_SILENCE_SUCCESS.format(duration=duration), ctx.channel, channel, True) + formatted_message = MSG_SILENCE_SUCCESS.format(duration=duration) + await self.send_message(formatted_message, ctx.channel, channel, alert_target=True) @commands.command(aliases=("unhush",)) async def unsilence(self, ctx: Context, *, channel: TextOrVoiceChannel = None) -> None: @@ -227,7 +229,7 @@ class Silence(commands.Cog): await self.send_message(MSG_UNSILENCE_FAIL, msg_channel, channel) else: - await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, True) + await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, alert_target=True) async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, kick: bool = False) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 31894761c..038e0a1a4 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -178,20 +178,20 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): mock_voice_channels.reset_mock() with self.subTest("Basic One Channel Test"): - await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, False) + await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, alert_target=False) text_channel_1.send.assert_called_once_with("Text basic message.") text_channel_2.send.assert_not_called() reset() with self.subTest("Basic Two Channel Test"): - await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, True) + await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, alert_target=True) text_channel_1.send.assert_called_once_with("Text basic message.") text_channel_2.send.assert_called_once_with("Text basic message.") reset() with self.subTest("Replacement One Channel Test"): message = "Current. The following should be replaced: current channel." - await self.cog.send_message(message, text_channel_1, text_channel_2, False) + await self.cog.send_message(message, text_channel_1, text_channel_2, alert_target=False) text_channel_1.send.assert_called_once_with(message.replace("current channel", text_channel_1.mention)) text_channel_2.send.assert_not_called() @@ -199,15 +199,16 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): reset() with self.subTest("Replacement Two Channel Test"): message = "Current. The following should be replaced: current channel." - await self.cog.send_message(message, text_channel_1, text_channel_2, True) + await self.cog.send_message(message, text_channel_1, text_channel_2, alert_target=True) text_channel_1.send.assert_called_once_with(message.replace("current channel", text_channel_1.mention)) text_channel_2.send.assert_called_once_with(message) reset() with self.subTest("Text and Voice"): - await self.cog.send_message("This should show up just here", text_channel_1, voice_channel, False) - text_channel_1.send.assert_called_once_with("This should show up just here") + message = "This should show up just here" + await self.cog.send_message(message, text_channel_1, voice_channel, alert_target=False) + text_channel_1.send.assert_called_once_with(message) with mock.patch.object(self.cog, "bot") as bot_mock: bot_mock.get_channel.return_value = text_channel_2 @@ -215,7 +216,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): reset() with self.subTest("Text and Voice"): message = "This should show up as current channel" - await self.cog.send_message(message, text_channel_1, voice_channel, True) + await self.cog.send_message(message, text_channel_1, voice_channel, alert_target=True) text_channel_1.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) @@ -226,7 +227,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): reset() with self.subTest("Text and Voice Same Invocation"): message = "This should show up as current channel" - await self.cog.send_message(message, text_channel_2, voice_channel, True) + await self.cog.send_message(message, text_channel_2, voice_channel, alert_target=True) text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) mock_voice_channels.get.assert_called_once_with(voice_channel.id) @@ -436,7 +437,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): with self.subTest(target_channel=target, message=message): with mock.patch.object(self.cog, "bot") as bot_mock: bot_mock.get_channel.return_value = AsyncMock() - await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) + await self.cog.silence.callback(self.cog, ctx, 10, target, kick=False) if ctx.channel == target or target is None: ctx.channel.send.assert_called_once_with(message) @@ -822,9 +823,9 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence_wrapper(channel, context) if context is None: - send_message.assert_called_once_with(message, channel, channel, True) + send_message.assert_called_once_with(message, channel, channel, alert_target=True) else: - send_message.assert_called_once_with(message, context.channel, channel, True) + send_message.assert_called_once_with(message, context.channel, channel, alert_target=True) channel.set_permissions.assert_called_once_with(role, overwrite=overwrites) if channel != ctx.channel: -- cgit v1.2.3 From f8afee54c3ffb1bfee3fca443738f4e91b3d1565 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 29 Dec 2020 19:12:53 +0300 Subject: Adds Error Handling For Voice Channel Muting Adds error handlers to allow voice channel muting to handle as many members as possible. --- bot/exts/moderation/silence.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 31103bc3e..157c150fd 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -277,7 +277,12 @@ class Silence(commands.Cog): for member in channel.members: if self._helper_role not in member.roles: - await member.move_to(None, reason="Kicking member from voice channel.") + try: + await member.move_to(None, reason="Kicking member from voice channel.") + log.debug(f"Kicked {member.name} from voice channel.") + except Exception as e: + log.debug(f"Failed to move {member.name}. Reason: {e}") + continue log.debug("Removed all members.") @@ -298,11 +303,15 @@ class Silence(commands.Cog): if self._helper_role in member.roles: continue - await member.move_to(afk_channel, reason="Muting VC member.") - log.debug(f"Moved {member.name} to afk channel.") + try: + await member.move_to(afk_channel, reason="Muting VC member.") + log.debug(f"Moved {member.name} to afk channel.") - await member.move_to(channel, reason="Muting VC member.") - log.debug(f"Moved {member.name} to original voice channel.") + await member.move_to(channel, reason="Muting VC member.") + log.debug(f"Moved {member.name} to original voice channel.") + except Exception as e: + log.debug(f"Failed to move {member.name}. Reason: {e}") + continue finally: # Delete VC channel if it was created. -- cgit v1.2.3 From 1b747fccd16d2667c6f4129a222cd9ea3eda5602 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 29 Dec 2020 19:18:03 +0300 Subject: Makes Kick Keyword Only Parameter --- bot/exts/moderation/silence.py | 4 ++-- tests/bot/exts/moderation/test_silence.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 157c150fd..e91e558ec 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -171,7 +171,7 @@ class Silence(commands.Cog): channel_info = f"#{channel} ({channel.id})" log.debug(f"{ctx.author} is silencing channel {channel_info}.") - if not await self._set_silence_overwrites(channel, kick): + if not await self._set_silence_overwrites(channel, kick=kick): log.info(f"Tried to silence channel {channel_info} but the channel was already silenced.") await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel) return @@ -231,7 +231,7 @@ class Silence(commands.Cog): else: await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, alert_target=True) - async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, kick: bool = False) -> bool: + async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, *, kick: bool = False) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" # Get the original channel overwrites if isinstance(channel, TextChannel): diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 038e0a1a4..44c3620ac 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -586,7 +586,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel)) self.assertFalse(self.voice_overwrite.speak) - self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel, True)) + self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel, kick=True)) self.assertFalse(self.voice_overwrite.speak or self.voice_overwrite.connect) -- cgit v1.2.3 From 9d3fba1c3143a529e32bd660696922c3ff902d16 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 29 Dec 2020 20:06:51 +0300 Subject: Breaks Out Send Message Tests Moves the tests for the helper method `send_message` to simplify tests, and avoid repeated code. --- tests/bot/exts/moderation/test_silence.py | 156 ++++++++++++++++-------------- 1 file changed, 81 insertions(+), 75 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 44c3620ac..8f4574d13 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -159,81 +159,6 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): role_check.assert_called_once_with(*(1, 2, 3)) role_check.return_value.predicate.assert_awaited_once_with(ctx) - @mock.patch.object(silence, "VOICE_CHANNELS") - async def test_send_message(self, mock_voice_channels): - """Test the send function reports to the correct channels.""" - text_channel_1 = MockTextChannel() - text_channel_2 = MockTextChannel() - - voice_channel = MockVoiceChannel() - voice_channel.name = "General/Offtopic" - voice_channel.mention = f"#{voice_channel.name}" - - mock_voice_channels.get.return_value = text_channel_2.id - - def reset(): - text_channel_1.reset_mock() - text_channel_2.reset_mock() - voice_channel.reset_mock() - mock_voice_channels.reset_mock() - - with self.subTest("Basic One Channel Test"): - await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, alert_target=False) - text_channel_1.send.assert_called_once_with("Text basic message.") - text_channel_2.send.assert_not_called() - - reset() - with self.subTest("Basic Two Channel Test"): - await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, alert_target=True) - text_channel_1.send.assert_called_once_with("Text basic message.") - text_channel_2.send.assert_called_once_with("Text basic message.") - - reset() - with self.subTest("Replacement One Channel Test"): - message = "Current. The following should be replaced: current channel." - await self.cog.send_message(message, text_channel_1, text_channel_2, alert_target=False) - - text_channel_1.send.assert_called_once_with(message.replace("current channel", text_channel_1.mention)) - text_channel_2.send.assert_not_called() - - reset() - with self.subTest("Replacement Two Channel Test"): - message = "Current. The following should be replaced: current channel." - await self.cog.send_message(message, text_channel_1, text_channel_2, alert_target=True) - - text_channel_1.send.assert_called_once_with(message.replace("current channel", text_channel_1.mention)) - text_channel_2.send.assert_called_once_with(message) - - reset() - with self.subTest("Text and Voice"): - message = "This should show up just here" - await self.cog.send_message(message, text_channel_1, voice_channel, alert_target=False) - text_channel_1.send.assert_called_once_with(message) - - with mock.patch.object(self.cog, "bot") as bot_mock: - bot_mock.get_channel.return_value = text_channel_2 - - reset() - with self.subTest("Text and Voice"): - message = "This should show up as current channel" - await self.cog.send_message(message, text_channel_1, voice_channel, alert_target=True) - text_channel_1.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - - mock_voice_channels.get.assert_called_once_with(voice_channel.id) - bot_mock.get_channel.assert_called_once_with(text_channel_2.id) - bot_mock.reset_mock() - - reset() - with self.subTest("Text and Voice Same Invocation"): - message = "This should show up as current channel" - await self.cog.send_message(message, text_channel_2, voice_channel, alert_target=True) - text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - - mock_voice_channels.get.assert_called_once_with(voice_channel.id) - bot_mock.get_channel.assert_called_once_with(text_channel_2.id) - bot_mock.reset_mock() - async def test_force_voice_sync(self): """Tests the _force_voice_sync helper function.""" await self.cog._async_init() @@ -832,3 +757,84 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): ctx.channel.send.assert_not_called() await reset() + + +class SendMessageTests(unittest.IsolatedAsyncioTestCase): + """Unittests for the send message helper function.""" + + def setUp(self) -> None: + self.bot = MockBot() + self.cog = silence.Silence(self.bot) + + self.text_channels = [MockTextChannel() for _ in range(2)] + self.bot.get_channel.return_value = self.text_channels[1] + + self.voice_channel = MockVoiceChannel(name="General/Offtopic") + + async def test_send_to_channel(self): + """Tests a basic case for the send function.""" + message = "Test basic message." + await self.cog.send_message(message, *self.text_channels, alert_target=False) + + self.text_channels[0].send.assert_called_once_with(message) + self.text_channels[1].send.assert_not_called() + + async def test_send_to_multiple_channels(self): + """Tests sending messages to two channels.""" + message = "Test basic message." + await self.cog.send_message(message, *self.text_channels, alert_target=True) + + self.text_channels[0].send.assert_called_once_with(message) + self.text_channels[1].send.assert_called_once_with(message) + + async def test_duration_replacement(self): + """Tests that the channel name was set correctly for one target channel.""" + message = "Current. The following should be replaced: current channel." + await self.cog.send_message(message, *self.text_channels, alert_target=False) + + updated_message = message.replace("current channel", self.text_channels[0].mention) + self.text_channels[0].send.assert_called_once_with(updated_message) + self.text_channels[1].send.assert_not_called() + + async def test_name_replacement_multiple_channels(self): + """Tests that the channel name was set correctly for two channels.""" + message = "Current. The following should be replaced: current channel." + await self.cog.send_message(message, *self.text_channels, alert_target=True) + + updated_message = message.replace("current channel", self.text_channels[0].mention) + self.text_channels[0].send.assert_called_once_with(updated_message) + self.text_channels[1].send.assert_called_once_with(message) + + async def test_silence_voice(self): + """Tests that the correct message was sent when a voice channel is muted without alerting.""" + message = "This should show up just here." + await self.cog.send_message(message, self.text_channels[0], self.voice_channel, alert_target=False) + self.text_channels[0].send.assert_called_once_with(message) + + async def test_silence_voice_alert(self): + """Tests that the correct message was sent when a voice channel is muted with alerts.""" + with unittest.mock.patch.object(silence, "VOICE_CHANNELS") as mock_voice_channels: + mock_voice_channels.get.return_value = self.text_channels[1].id + + message = "This should show up as current channel." + await self.cog.send_message(message, self.text_channels[0], self.voice_channel, alert_target=True) + + updated_message = message.replace("current channel", self.voice_channel.mention) + self.text_channels[0].send.assert_called_once_with(updated_message) + self.text_channels[1].send.assert_called_once_with(updated_message) + + mock_voice_channels.get.assert_called_once_with(self.voice_channel.id) + + async def test_silence_voice_sibling_channel(self): + """Tests silencing a voice channel from the related text channel.""" + with unittest.mock.patch.object(silence, "VOICE_CHANNELS") as mock_voice_channels: + mock_voice_channels.get.return_value = self.text_channels[1].id + + message = "This should show up as current channel." + await self.cog.send_message(message, self.text_channels[1], self.voice_channel, alert_target=True) + + updated_message = message.replace("current channel", self.voice_channel.mention) + self.text_channels[1].send.assert_called_once_with(updated_message) + + mock_voice_channels.get.assert_called_once_with(self.voice_channel.id) + self.bot.get_channel.assert_called_once_with(self.text_channels[1].id) -- cgit v1.2.3 From 43c407cc925ea27faab93d86e54bfce5cca8a8b1 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sun, 10 Jan 2021 16:16:34 +0300 Subject: Improves Unsilence Wrapper Docstring Modifies the unsilence wrapper docstring to make the arguments and behavior more clear. --- bot/exts/moderation/silence.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index e91e558ec..26aa77b61 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -209,7 +209,12 @@ class Silence(commands.Cog): @lock_arg(LOCK_NAMESPACE, "channel", raise_error=True) async def _unsilence_wrapper(self, channel: TextOrVoiceChannel, ctx: Optional[Context] = None) -> None: - """Unsilence `channel` and send a success/failure message.""" + """ + Unsilence `channel` and send a success/failure message to ctx.channel. + + If ctx is None or not passed, `channel` is used in its place. + If `channel` and ctx.channel are the same, only one message is sent. + """ msg_channel = channel if ctx is not None: msg_channel = ctx.channel -- cgit v1.2.3 From 70fcd2b68706ae5e8e35407beaa04e2895f3dae8 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sun, 10 Jan 2021 16:22:15 +0300 Subject: Cleans Up & Simplifies Tests Cleans up the silence tests by removing unneeded or repeated mocks. Simplifies tests where possible by joining similar tests. --- bot/exts/moderation/silence.py | 4 +- tests/bot/exts/moderation/test_silence.py | 111 +++++++++--------------------- 2 files changed, 35 insertions(+), 80 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 26aa77b61..1a3c48394 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -229,9 +229,9 @@ class Silence(commands.Cog): # Send fail message to muted channel or voice chat channel, and invocation channel if manual: - await self.send_message(MSG_UNSILENCE_MANUAL, msg_channel, channel) + await self.send_message(MSG_UNSILENCE_MANUAL, msg_channel, channel, alert_target=False) else: - await self.send_message(MSG_UNSILENCE_FAIL, msg_channel, channel) + await self.send_message(MSG_UNSILENCE_FAIL, msg_channel, channel, alert_target=False) else: await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, alert_target=True) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 8f4574d13..5505d7a53 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -163,29 +163,22 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): """Tests the _force_voice_sync helper function.""" await self.cog._async_init() - members = [] - for _ in range(10): - members.append(MockMember()) + members = [MockMember() for _ in range(10)] + members.extend([MockMember(roles=[self.cog._helper_role]) for _ in range(3)]) - afk_channel = MockVoiceChannel() - channel = MockVoiceChannel(guild=MockGuild(afk_channel=afk_channel), members=members) + channel = MockVoiceChannel(members=members) await self.cog._force_voice_sync(channel) for member in members: - self.assertEqual(member.move_to.call_count, 2) - calls = [ - mock.call(afk_channel, reason="Muting VC member."), - mock.call(channel, reason="Muting VC member.") - ] - member.move_to.assert_has_calls(calls, any_order=False) - - async def test_force_voice_sync_staff(self): - """Tests to ensure _force_voice_sync does not kick staff members.""" - await self.cog._async_init() - member = MockMember(roles=[self.cog._helper_role]) + if self.cog._helper_role in member.roles: + member.move_to.assert_not_called() + else: + self.assertEqual(member.move_to.call_count, 2) + calls = member.move_to.call_args_list - await self.cog._force_voice_sync(MockVoiceChannel(members=[member])) - member.move_to.assert_not_called() + # Tests that the member was moved to the afk channel, and back. + self.assertEqual((channel.guild.afk_channel,), calls[0].args) + self.assertEqual((channel,), calls[1].args) async def test_force_voice_sync_no_channel(self): """Test to ensure _force_voice_sync can create its own voice channel if one is not available.""" @@ -204,29 +197,23 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): channel.guild.create_voice_channel.assert_called_once_with("mute-temp", overwrites=overwrites) # Check bot deleted channel - new_channel.delete.assert_called_once_with(reason="Deleting temp mute channel.") + new_channel.delete.assert_called_once() async def test_voice_kick(self): """Test to ensure kick function can remove all members from a voice channel.""" await self.cog._async_init() - members = [] - for _ in range(10): - members.append(MockMember()) + members = [MockMember() for _ in range(10)] + members.extend([MockMember(roles=[self.cog._helper_role]) for _ in range(3)]) channel = MockVoiceChannel(members=members) await self.cog._kick_voice_members(channel) for member in members: - member.move_to.assert_called_once_with(None, reason="Kicking member from voice channel.") - - async def test_voice_kick_staff(self): - """Test to ensure voice kick skips staff members.""" - await self.cog._async_init() - member = MockMember(roles=[self.cog._helper_role]) - - await self.cog._kick_voice_members(MockVoiceChannel(members=[member])) - member.move_to.assert_not_called() + if self.cog._helper_role in member.roles: + member.move_to.assert_not_called() + else: + self.assertEqual((None,), member.move_to.call_args_list[0].args) @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) @@ -311,7 +298,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): @autospec(silence.Silence, "_reschedule", pass_mocks=False) @autospec(silence, "Scheduler", "SilenceNotifier", pass_mocks=False) def setUp(self) -> None: - self.bot = MockBot() + self.bot = MockBot(get_channel=lambda _: MockTextChannel()) self.cog = silence.Silence(self.bot) self.cog._init_task = asyncio.Future() self.cog._init_task.set_result(None) @@ -360,9 +347,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): for target, message in test_cases: with self.subTest(target_channel=target, message=message): - with mock.patch.object(self.cog, "bot") as bot_mock: - bot_mock.get_channel.return_value = AsyncMock() - await self.cog.silence.callback(self.cog, ctx, 10, target, kick=False) + await self.cog.silence.callback(self.cog, ctx, 10, target, kick=False) if ctx.channel == target or target is None: ctx.channel.send.assert_called_once_with(message) @@ -505,9 +490,6 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_correct_permission_updates(self): """Tests if _set_silence_overwrites can correctly get and update permissions.""" - self.assertTrue(await self.cog._set_silence_overwrites(self.text_channel)) - self.assertFalse(self.text_overwrite.send_messages or self.text_overwrite.add_reactions) - self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel)) self.assertFalse(self.voice_overwrite.speak) @@ -548,49 +530,22 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): (False, silence.MSG_UNSILENCE_MANUAL, PermissionOverwrite(send_messages=False)), (False, silence.MSG_UNSILENCE_MANUAL, PermissionOverwrite(add_reactions=False)), ) + for was_unsilenced, message, overwrite in test_cases: ctx = MockContext() - with self.subTest(was_unsilenced=was_unsilenced, message=message, overwrite=overwrite): - with mock.patch.object(self.cog, "_unsilence", return_value=was_unsilenced): - ctx.channel.overwrites_for.return_value = overwrite - await self.cog.unsilence.callback(self.cog, ctx) - ctx.channel.send.assert_called_once_with(message) - - async def test_sent_to_correct_channel(self): - """Test function sends messages to the correct channels.""" - unsilenced_overwrite = PermissionOverwrite(send_messages=True, add_reactions=True) - text_channel = MockTextChannel() - ctx = MockContext() - - test_cases = ( - (None, silence.MSG_UNSILENCE_SUCCESS.format(duration=10)), - (text_channel, silence.MSG_UNSILENCE_SUCCESS.format(duration=10)), - (ctx.channel, silence.MSG_UNSILENCE_SUCCESS.format(duration=10)), - ) - for target, message in test_cases: - with self.subTest(target_channel=target, message=message): - with mock.patch.object(self.cog, "_unsilence", return_value=True): - # Assign Return - if ctx.channel == target or target is None: - ctx.channel.overwrites_for.return_value = unsilenced_overwrite - else: - target.overwrites_for.return_value = unsilenced_overwrite + for target in [None, MockTextChannel()]: + ctx.channel.overwrites_for.return_value = overwrite + if target: + target.overwrites_for.return_value = overwrite - await self.cog.unsilence.callback(self.cog, ctx, channel=target) + with mock.patch.object(self.cog, "_unsilence", return_value=was_unsilenced): + with mock.patch.object(self.cog, "send_message") as send_message: + with self.subTest(was_unsilenced=was_unsilenced, overwrite=overwrite, target=target): + await self.cog.unsilence.callback(self.cog, ctx, channel=target) - # Check Messages - if ctx.channel == target or target is None: - ctx.channel.send.assert_called_once_with(message) - else: - ctx.channel.send.assert_called_once_with( - message.replace("current channel", text_channel.mention) - ) - target.send.assert_called_once_with(message) - - ctx.channel.send.reset_mock() - if target is not None: - target.send.reset_mock() + call_args = (message, ctx.channel, target or ctx.channel) + send_message.assert_called_once_with(*call_args, alert_target=was_unsilenced) async def test_skipped_already_unsilenced(self): """Permissions were not set and `False` was returned for an already unsilenced channel.""" @@ -708,7 +663,7 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence_wrapper(channel, context) overwrites.assert_called_once_with(role) - send_message.assert_called_once_with(message, ctx.channel, channel) + send_message.assert_called_once_with(message, ctx.channel, channel, alert_target=False) send_message.reset_mock() @@ -769,7 +724,7 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): self.text_channels = [MockTextChannel() for _ in range(2)] self.bot.get_channel.return_value = self.text_channels[1] - self.voice_channel = MockVoiceChannel(name="General/Offtopic") + self.voice_channel = MockVoiceChannel() async def test_send_to_channel(self): """Tests a basic case for the send function.""" -- cgit v1.2.3 From 52c69c5d51d9938cd7a56bf5cbc2c26a371883d9 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sun, 10 Jan 2021 16:23:32 +0300 Subject: Cleans Up Voice Sync Tests Cleans up the tests related to the voice sync/kick functions by adding a helper method to simplify mocking. --- tests/bot/exts/moderation/test_silence.py | 59 +++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 5505d7a53..a365b2aae 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -291,6 +291,17 @@ class RescheduleTests(unittest.IsolatedAsyncioTestCase): self.cog.notifier.add_channel.assert_not_called() +def voice_sync_helper(function): + """Helper wrapper to test the sync and kick functions for voice channels.""" + @autospec(silence.Silence, "_force_voice_sync", "_kick_voice_members", "_set_silence_overwrites") + async def inner(self, sync, kick, overwrites): + overwrites.return_value = True + await function(self, MockContext(), + sync, kick) + + return inner + + @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) class SilenceTests(unittest.IsolatedAsyncioTestCase): """Tests for the silence command and its related helper methods.""" @@ -363,29 +374,41 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): if target is not None and isinstance(target, MockTextChannel): target.send.reset_mock() - @mock.patch.object(silence.Silence, "_set_silence_overwrites", return_value=True) - @mock.patch.object(silence.Silence, "_kick_voice_members") - @mock.patch.object(silence.Silence, "_force_voice_sync") - async def test_sync_or_kick_called(self, sync, kick, _): - """Tests if silence command calls kick or sync on voice channels when appropriate.""" + @voice_sync_helper + async def test_sync_called(self, ctx, sync, kick): + """Tests if silence command calls sync on a voice channel.""" channel = MockVoiceChannel() - ctx = MockContext() + await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=False) - with mock.patch.object(self.cog, "bot") as bot_mock: - bot_mock.get_channel.return_value = AsyncMock() + sync.assert_called_once_with(self.cog, channel) + kick.assert_not_called() - with self.subTest("Test calls kick"): - await self.cog.silence.callback(self.cog, ctx, 10, kick=True, channel=channel) - kick.assert_called_once_with(channel) - sync.assert_not_called() + @voice_sync_helper + async def test_kick_called(self, ctx, sync, kick): + """Tests if silence command calls kick on a voice channel.""" + channel = MockVoiceChannel() + await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=True) + + kick.assert_called_once_with(self.cog, channel) + sync.assert_not_called() + + @voice_sync_helper + async def test_sync_not_called(self, ctx, sync, kick): + """Tests that silence command does not call sync on a text channel.""" + channel = MockTextChannel() + await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=False) + + sync.assert_not_called() + kick.assert_not_called() - kick.reset_mock() - sync.reset_mock() + @voice_sync_helper + async def test_kick_not_called(self, ctx, sync, kick): + """Tests that silence command does not call kick on a text channel.""" + channel = MockTextChannel() + await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=True) - with self.subTest("Test calls sync"): - await self.cog.silence.callback(self.cog, ctx, 10, kick=False, channel=channel) - sync.assert_called_once_with(channel) - kick.assert_not_called() + sync.assert_not_called() + kick.assert_not_called() async def test_skipped_already_silenced(self): """Permissions were not set and `False` was returned for an already silenced channel.""" -- cgit v1.2.3 From 1c08e5a152a3ac0f571bdbd6f6c632f9299dda03 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Thu, 4 Feb 2021 11:17:52 +0300 Subject: Updates Voice Channel Config Adds the new voice and text channels to the default config, and renames all affected channels in the config and constants to match the new names. --- bot/constants.py | 10 ++++++---- bot/exts/moderation/silence.py | 5 +++-- config-default.yml | 14 ++++++++------ 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/bot/constants.py b/bot/constants.py index 95e22513f..e0ff80e93 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -441,15 +441,17 @@ class Channels(metaclass=YAMLGetter): staff_announcements: int admins_voice: int + code_help_voice_0: int code_help_voice_1: int - code_help_voice_2: int - general_voice: int + general_voice_0: int + general_voice_1: int staff_voice: int + code_help_chat_0: int code_help_chat_1: int - code_help_chat_2: int staff_voice_chat: int - voice_chat: int + voice_chat_0: int + voice_chat_1: int big_brother_logs: int talent_pool: int diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index cf4d16067..c15cbccaa 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -34,9 +34,10 @@ MSG_UNSILENCE_SUCCESS = f"{constants.Emojis.check_mark} unsilenced current chann TextOrVoiceChannel = Union[TextChannel, VoiceChannel] VOICE_CHANNELS = { + constants.Channels.code_help_voice_0: constants.Channels.code_help_chat_0, constants.Channels.code_help_voice_1: constants.Channels.code_help_chat_1, - constants.Channels.code_help_voice_2: constants.Channels.code_help_chat_2, - constants.Channels.general_voice: constants.Channels.voice_chat, + constants.Channels.general_voice_0: constants.Channels.voice_chat_0, + constants.Channels.general_voice_1: constants.Channels.voice_chat_1, constants.Channels.staff_voice: constants.Channels.staff_voice_chat, } diff --git a/config-default.yml b/config-default.yml index d3b267159..8e8d1f43e 100644 --- a/config-default.yml +++ b/config-default.yml @@ -204,16 +204,18 @@ guild: # Voice Channels admins_voice: &ADMINS_VOICE 500734494840717332 - code_help_voice_1: 751592231726481530 - code_help_voice_2: 764232549840846858 - general_voice: 751591688538947646 + code_help_voice_0: 751592231726481530 + code_help_voice_1: 764232549840846858 + general_voice_0: 751591688538947646 + general_voice_1: 799641437645701151 staff_voice: &STAFF_VOICE 412375055910043655 # Voice Chat - code_help_chat_1: 755154969761677312 - code_help_chat_2: 766330079135268884 + code_help_chat_0: 755154969761677312 + code_help_chat_1: 766330079135268884 staff_voice_chat: 541638762007101470 - voice_chat: 412357430186344448 + voice_chat_0: 412357430186344448 + voice_chat_1: 799647045886541885 # Watch big_brother_logs: &BB_LOGS 468507907357409333 -- cgit v1.2.3 From b10e9b0e3f7de0daf52234f336e43154aa4c1e9a Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Thu, 4 Feb 2021 11:54:35 +0300 Subject: Restricts Voice Silence Skip To Mod Roles Raises the permission required to not be muted during a voice silence to moderation roles. --- bot/exts/moderation/silence.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index c15cbccaa..4253cd4f3 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -282,13 +282,16 @@ class Silence(commands.Cog): log.debug(f"Removing all non staff members from #{channel.name} ({channel.id}).") for member in channel.members: - if self._helper_role not in member.roles: - try: - await member.move_to(None, reason="Kicking member from voice channel.") - log.debug(f"Kicked {member.name} from voice channel.") - except Exception as e: - log.debug(f"Failed to move {member.name}. Reason: {e}") - continue + # Skip staff + if any(role.id in constants.MODERATION_ROLES for role in member.roles): + continue + + try: + await member.move_to(None, reason="Kicking member from voice channel.") + log.debug(f"Kicked {member.name} from voice channel.") + except Exception as e: + log.debug(f"Failed to move {member.name}. Reason: {e}") + continue log.debug("Removed all members.") @@ -306,7 +309,7 @@ class Silence(commands.Cog): # Move all members to temporary channel and back for member in channel.members: # Skip staff - if self._helper_role in member.roles: + if any(role.id in constants.MODERATION_ROLES for role in member.roles): continue try: -- cgit v1.2.3 From a9034e649722730ed2551c2660e8d03f798f25a1 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Thu, 4 Feb 2021 11:58:37 +0300 Subject: Modifies Channel Creation Reason Co-authored-by: Matteo Bertucci --- bot/exts/moderation/silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 4253cd4f3..d0115b0cd 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -325,7 +325,7 @@ class Silence(commands.Cog): finally: # Delete VC channel if it was created. if delete_channel: - await afk_channel.delete(reason="Deleting temp mute channel.") + await afk_channel.delete(reason="Deleting temporary mute channel.") async def _schedule_unsilence(self, ctx: Context, channel: TextOrVoiceChannel, duration: Optional[int]) -> None: """Schedule `ctx.channel` to be unsilenced if `duration` is not None.""" -- cgit v1.2.3 From 1a1a283d617592fbf3995d7cd5e1d88be75f92ea Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Thu, 4 Feb 2021 12:29:59 +0300 Subject: Updates Voice Kick Restriction Tests --- bot/exts/moderation/silence.py | 4 ++-- tests/bot/exts/moderation/test_silence.py | 26 +++++++++++++++++--------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 4253cd4f3..ea531c37a 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -117,7 +117,6 @@ class Silence(commands.Cog): self._everyone_role = guild.default_role self._verified_voice_role = guild.get_role(constants.Roles.voice_verified) - self._helper_role = guild.get_role(constants.Roles.helpers) self._mod_alerts_channel = self.bot.get_channel(constants.Channels.mod_alerts) @@ -277,7 +276,8 @@ class Silence(commands.Cog): return afk_channel - async def _kick_voice_members(self, channel: VoiceChannel) -> None: + @staticmethod + async def _kick_voice_members(channel: VoiceChannel) -> None: """Remove all non-staff members from a voice channel.""" log.debug(f"Removing all non staff members from #{channel.name} ({channel.id}).") diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index a365b2aae..2d85af7e0 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -7,9 +7,18 @@ from unittest.mock import AsyncMock, Mock from async_rediscache import RedisSession from discord import PermissionOverwrite -from bot.constants import Channels, Guild, Roles +from bot.constants import Channels, Guild, MODERATION_ROLES, Roles from bot.exts.moderation import silence -from tests.helpers import MockBot, MockContext, MockGuild, MockMember, MockTextChannel, MockVoiceChannel, autospec +from tests.helpers import ( + MockBot, + MockContext, + MockGuild, + MockMember, + MockRole, + MockTextChannel, + MockVoiceChannel, + autospec +) redis_session = None redis_loop = asyncio.get_event_loop() @@ -164,13 +173,13 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): await self.cog._async_init() members = [MockMember() for _ in range(10)] - members.extend([MockMember(roles=[self.cog._helper_role]) for _ in range(3)]) + members.extend([MockMember(roles=[MockRole(id=role)]) for role in MODERATION_ROLES]) channel = MockVoiceChannel(members=members) await self.cog._force_voice_sync(channel) for member in members: - if self.cog._helper_role in member.roles: + if any(role.id in MODERATION_ROLES for role in member.roles): member.move_to.assert_not_called() else: self.assertEqual(member.move_to.call_count, 2) @@ -204,13 +213,13 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): await self.cog._async_init() members = [MockMember() for _ in range(10)] - members.extend([MockMember(roles=[self.cog._helper_role]) for _ in range(3)]) + members.extend([MockMember(roles=[MockRole(id=role)]) for role in MODERATION_ROLES]) channel = MockVoiceChannel(members=members) await self.cog._kick_voice_members(channel) for member in members: - if self.cog._helper_role in member.roles: + if any(role.id in MODERATION_ROLES for role in member.roles): member.move_to.assert_not_called() else: self.assertEqual((None,), member.move_to.call_args_list[0].args) @@ -296,8 +305,7 @@ def voice_sync_helper(function): @autospec(silence.Silence, "_force_voice_sync", "_kick_voice_members", "_set_silence_overwrites") async def inner(self, sync, kick, overwrites): overwrites.return_value = True - await function(self, MockContext(), - sync, kick) + await function(self, MockContext(), sync, kick) return inner @@ -389,7 +397,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockVoiceChannel() await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=True) - kick.assert_called_once_with(self.cog, channel) + kick.assert_called_once_with(channel) sync.assert_not_called() @voice_sync_helper -- cgit v1.2.3 From baa907f5ef056ec3001759cc6f9a9523953afc39 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Thu, 4 Feb 2021 13:07:16 +0300 Subject: Adds Move To Failure Tests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 2d85af7e0..70fe756fd 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -224,6 +224,32 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): else: self.assertEqual((None,), member.move_to.call_args_list[0].args) + async def test_voice_move_to_error(self): + """Test to ensure move_to get called on all members, even if some fail.""" + await self.cog._async_init() + + def failing_move_to(*_): + raise Exception() + failing_members = [MockMember(move_to=Mock(failing_move_to)) for _ in range(5)] + + members = [] + for i in range(5): + members.append(MockMember()) + members.append(failing_members[i]) + + channel = MockVoiceChannel(members=members) + + with self.subTest("Kick"): + await self.cog._kick_voice_members(channel) + for member in members: + member.move_to.assert_called_once() + member.reset_mock() + + with self.subTest("Sync"): + await self.cog._force_voice_sync(channel) + for member in members: + self.assertEqual(member.move_to.call_count, 1 if member in failing_members else 2) + @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) class RescheduleTests(unittest.IsolatedAsyncioTestCase): -- cgit v1.2.3 From 09886c234c9840dc2c2eca0f0a26e72ae6cee527 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Thu, 4 Feb 2021 19:58:50 +0300 Subject: Separates Voice Overwrite Tests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 70fe756fd..6b48792cb 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -461,7 +461,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.assertFalse(await self.cog._set_silence_overwrites(channel)) channel.set_permissions.assert_not_called() - async def test_silenced_channel(self): + async def test_silenced_text_channel(self): """Channel had `send_message` and `add_reactions` permissions revoked for verified role.""" self.assertTrue(await self.cog._set_silence_overwrites(self.text_channel)) self.assertFalse(self.text_overwrite.send_messages) @@ -471,6 +471,24 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): overwrite=self.text_overwrite ) + async def test_silenced_voice_channel_speak(self): + """Channel had `speak` permissions revoked for verified role.""" + self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel)) + self.assertFalse(self.voice_overwrite.speak) + self.voice_channel.set_permissions.assert_awaited_once_with( + self.cog._verified_voice_role, + overwrite=self.voice_overwrite + ) + + async def test_silenced_voice_channel_full(self): + """Channel had `speak` and `connect` permissions revoked for verified role.""" + self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel, kick=True)) + self.assertFalse(self.voice_overwrite.speak or self.voice_overwrite.connect) + self.voice_channel.set_permissions.assert_awaited_once_with( + self.cog._verified_voice_role, + overwrite=self.voice_overwrite + ) + async def test_preserved_other_overwrites(self): """Channel's other unrelated overwrites were not changed.""" prev_overwrite_dict = dict(self.text_overwrite) @@ -545,14 +563,6 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.silence.callback(self.cog, ctx, None) self.cog.scheduler.schedule_later.assert_not_called() - async def test_correct_permission_updates(self): - """Tests if _set_silence_overwrites can correctly get and update permissions.""" - self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel)) - self.assertFalse(self.voice_overwrite.speak) - - self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel, kick=True)) - self.assertFalse(self.voice_overwrite.speak or self.voice_overwrite.connect) - @autospec(silence.Silence, "unsilence_timestamps", pass_mocks=False) class UnsilenceTests(unittest.IsolatedAsyncioTestCase): -- cgit v1.2.3 From 4fea67cb8a535228cb37a83c4a2d44b5112fb707 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Fri, 5 Feb 2021 02:23:10 +0300 Subject: Modifies Silence Tests Adds a missing test assertion, and seperates the voice and text components of a test. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 6b48792cb..2fcf4de43 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -648,14 +648,15 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): self.assertIsNone(self.overwrite.send_messages) self.assertIsNone(self.overwrite.add_reactions) - async def test_cache_miss_sent_mod_alert(self): - """A message was sent to the mod alerts channel.""" + async def test_cache_miss_sent_mod_alert_text(self): + """A message was sent to the mod alerts channel upon muting a text channel.""" self.cog.previous_overwrites.get.return_value = None - await self.cog._unsilence(self.channel) self.cog._mod_alerts_channel.send.assert_awaited_once() - self.cog._mod_alerts_channel.send.reset_mock() + async def test_cache_miss_sent_mod_alert_voice(self): + """A message was sent to the mod alerts channel upon muting a voice channel.""" + self.cog.previous_overwrites.get.return_value = None await self.cog._unsilence(MockVoiceChannel()) self.cog._mod_alerts_channel.send.assert_awaited_once() @@ -832,6 +833,7 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): message = "This should show up just here." await self.cog.send_message(message, self.text_channels[0], self.voice_channel, alert_target=False) self.text_channels[0].send.assert_called_once_with(message) + self.text_channels[1].send.assert_not_called() async def test_silence_voice_alert(self): """Tests that the correct message was sent when a voice channel is muted with alerts.""" -- cgit v1.2.3 From 1343a1c22bcd8a21c2d3fc38293033f546c86036 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Fri, 5 Feb 2021 02:49:06 +0300 Subject: Modifies Silence Tests Adds a missing test assertion, and seperates the voice and text components of a test. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 43 ++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 2fcf4de43..a52f2447d 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -1,6 +1,7 @@ import asyncio import unittest from datetime import datetime, timezone +from typing import List, Tuple from unittest import mock from unittest.mock import AsyncMock, Mock @@ -224,31 +225,43 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): else: self.assertEqual((None,), member.move_to.call_args_list[0].args) - async def test_voice_move_to_error(self): - """Test to ensure move_to get called on all members, even if some fail.""" - await self.cog._async_init() + @staticmethod + def create_erroneous_members() -> Tuple[List[MockMember], List[MockMember]]: + """ + Helper method to generate a list of members that error out on move_to call. + Returns the list of erroneous members, + as well as a list of regular and erroneous members combined, in that order. + """ def failing_move_to(*_): raise Exception() - failing_members = [MockMember(move_to=Mock(failing_move_to)) for _ in range(5)] + erroneous_members = [MockMember(move_to=Mock(failing_move_to)) for _ in range(5)] members = [] for i in range(5): members.append(MockMember()) - members.append(failing_members[i]) + members.append(erroneous_members[i]) - channel = MockVoiceChannel(members=members) + return erroneous_members, members + + async def test_kick_move_to_error(self): + """Test to ensure move_to gets called on all members during kick, even if some fail.""" + await self.cog._async_init() + failing_members, members = self.create_erroneous_members() - with self.subTest("Kick"): - await self.cog._kick_voice_members(channel) - for member in members: - member.move_to.assert_called_once() - member.reset_mock() + await self.cog._kick_voice_members(MockVoiceChannel(members=members)) + for member in members: + member.move_to.assert_called_once() + member.reset_mock() + + async def test_sync_move_to_error(self): + """Test to ensure move_to gets called on all members during sync, even if some fail.""" + await self.cog._async_init() + failing_members, members = self.create_erroneous_members() - with self.subTest("Sync"): - await self.cog._force_voice_sync(channel) - for member in members: - self.assertEqual(member.move_to.call_count, 1 if member in failing_members else 2) + await self.cog._force_voice_sync(MockVoiceChannel(members=members)) + for member in members: + self.assertEqual(member.move_to.call_count, 1 if member in failing_members else 2) @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) -- cgit v1.2.3 From acf2b643672703d2bc2011558e9fb68c76d0bc17 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Wed, 10 Mar 2021 20:36:36 +0300 Subject: Combine Silence Target Tests Combine two tests that are responsible for checking the silence helper uses the correct channel and message. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 2 +- tests/bot/exts/moderation/test_silence.py | 53 +++++++++---------------------- 2 files changed, 16 insertions(+), 39 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index dd379b412..616dfbefb 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -173,7 +173,7 @@ class Silence(commands.Cog): if not await self._set_silence_overwrites(channel, kick=kick): log.info(f"Tried to silence channel {channel_info} but the channel was already silenced.") - await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel) + await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel, alert_target=False) return if isinstance(channel, VoiceChannel): diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index a52f2447d..c3b30450f 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -375,51 +375,28 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.voice_channel.overwrites_for.return_value = self.voice_overwrite async def test_sent_correct_message(self): - """Appropriate failure/success message was sent by the command.""" + """Appropriate failure/success message was sent by the command to the correct channel.""" + # The following test tuples are made up of: + # duration, expected message, and the success of the _set_silence_overwrites function test_cases = ( (0.0001, silence.MSG_SILENCE_SUCCESS.format(duration=0.0001), True,), (None, silence.MSG_SILENCE_PERMANENT, True,), (5, silence.MSG_SILENCE_FAIL, False,), ) + for duration, message, was_silenced in test_cases: - ctx = MockContext() with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=was_silenced): - with self.subTest(was_silenced=was_silenced, message=message, duration=duration): - await self.cog.silence.callback(self.cog, ctx, duration) - ctx.channel.send.assert_called_once_with(message) - - @mock.patch.object(silence.Silence, "_set_silence_overwrites", return_value=True) - @mock.patch.object(silence.Silence, "_force_voice_sync") - async def test_sent_to_correct_channel(self, voice_sync, _): - """Test function sends messages to the correct channels.""" - text_channel = MockTextChannel() - voice_channel = MockVoiceChannel() - ctx = MockContext() - - test_cases = ( - (None, silence.MSG_SILENCE_SUCCESS.format(duration=10)), - (text_channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), - (voice_channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), - (ctx.channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), - ) - - for target, message in test_cases: - with self.subTest(target_channel=target, message=message): - await self.cog.silence.callback(self.cog, ctx, 10, target, kick=False) - - if ctx.channel == target or target is None: - ctx.channel.send.assert_called_once_with(message) - - else: - ctx.channel.send.assert_called_once_with(message.replace("current channel", target.mention)) - if isinstance(target, MockTextChannel): - target.send.assert_called_once_with(message) - else: - voice_sync.assert_called_once_with(target) - - ctx.channel.send.reset_mock() - if target is not None and isinstance(target, MockTextChannel): - target.send.reset_mock() + for target in [MockTextChannel(), MockVoiceChannel(), None]: + with self.subTest(was_silenced=was_silenced, target=target, message=message): + with mock.patch.object(self.cog, "send_message") as send_message: + ctx = MockContext() + await self.cog.silence.callback(self.cog, ctx, duration, target) + send_message.assert_called_once_with( + message, + ctx.channel, + target or ctx.channel, + alert_target=was_silenced + ) @voice_sync_helper async def test_sync_called(self, ctx, sync, kick): -- cgit v1.2.3 From e62707dfaf2dd833ff057ee472472d27a86ac223 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Wed, 10 Mar 2021 20:55:24 +0300 Subject: Simplifies Redundant Unsilence Target Test Removes redundant functionality from the `test_unsilence_helper_fail` test as it is covered by another test. Keeps the functionality that isn't being tested elsewhere. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 41 ++++++------------------------- 1 file changed, 8 insertions(+), 33 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index c3b30450f..5f2e67ac2 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -688,42 +688,17 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) - @mock.patch.object(silence.Silence, "_unsilence", return_value=False) - @mock.patch.object(silence.Silence, "send_message") - async def test_unsilence_helper_fail(self, send_message, _): - """Tests unsilence_wrapper when `_unsilence` fails.""" - ctx = MockContext() - - text_channel = MockTextChannel() - text_role = self.cog.bot.get_guild(Guild.id).default_role - - voice_channel = MockVoiceChannel() - voice_role = self.cog.bot.get_guild(Guild.id).get_role(Roles.voice_verified) - + async def test_unsilence_role(self): + """Tests unsilence_wrapper applies permission to the correct role.""" test_cases = ( - (ctx, text_channel, text_role, True, silence.MSG_UNSILENCE_FAIL), - (ctx, text_channel, text_role, False, silence.MSG_UNSILENCE_MANUAL), - (ctx, voice_channel, voice_role, True, silence.MSG_UNSILENCE_FAIL), - (ctx, voice_channel, voice_role, False, silence.MSG_UNSILENCE_MANUAL), + (MockTextChannel(), self.cog.bot.get_guild(Guild.id).default_role), + (MockVoiceChannel(), self.cog.bot.get_guild(Guild.id).get_role(Roles.voice_verified)) ) - class PermClass: - """Class to Mock return permissions""" - def __init__(self, value: bool): - self.val = value - - def __getattr__(self, item): - return self.val - - for context, channel, role, permission, message in test_cases: - with mock.patch.object(channel, "overwrites_for", return_value=PermClass(permission)) as overwrites: - with self.subTest(channel=channel, message=message): - await self.cog._unsilence_wrapper(channel, context) - - overwrites.assert_called_once_with(role) - send_message.assert_called_once_with(message, ctx.channel, channel, alert_target=False) - - send_message.reset_mock() + for channel, role in test_cases: + with self.subTest(channel=channel, role=role): + await self.cog._unsilence_wrapper(channel, MockContext()) + channel.overwrites_for.assert_called_with(role) @mock.patch.object(silence.Silence, "_force_voice_sync") @mock.patch.object(silence.Silence, "send_message") -- cgit v1.2.3 From b867fb21b6a14f91aa608009aa7eef540a4792dc Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Wed, 10 Mar 2021 22:38:59 +0300 Subject: Use Mock Side Effect Instead Of Extra Function Changes the mock used for creating an erroneous function in the silence tests cog to use the side effect property instead of an extra function. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 5f2e67ac2..a297cc8cb 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -233,9 +233,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): Returns the list of erroneous members, as well as a list of regular and erroneous members combined, in that order. """ - def failing_move_to(*_): - raise Exception() - erroneous_members = [MockMember(move_to=Mock(failing_move_to)) for _ in range(5)] + erroneous_members = [MockMember(move_to=Mock(side_effect=Exception())) for _ in range(5)] members = [] for i in range(5): -- cgit v1.2.3 From b0381d7164ceba20784fc244d724962ec35a9b13 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Fri, 12 Mar 2021 22:58:28 +0300 Subject: Removes Unused Mock Reset Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index a297cc8cb..d7542c562 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -250,7 +250,6 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): await self.cog._kick_voice_members(MockVoiceChannel(members=members)) for member in members: member.move_to.assert_called_once() - member.reset_mock() async def test_sync_move_to_error(self): """Test to ensure move_to gets called on all members during sync, even if some fail.""" -- cgit v1.2.3 From 2446c1728c3ba660d33b686d93d62a93debc74d2 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Wed, 21 Apr 2021 00:25:51 +0300 Subject: Removes Unnecessary Members In Silence Tests Reduces the number of members created for each test to the bare minimum required. Signed-off-by: Hassan Abouelela --- tests/bot/exts/moderation/test_silence.py | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index d7542c562..459048f68 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -173,14 +173,15 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): """Tests the _force_voice_sync helper function.""" await self.cog._async_init() - members = [MockMember() for _ in range(10)] - members.extend([MockMember(roles=[MockRole(id=role)]) for role in MODERATION_ROLES]) + # Create a regular member, and one member for each of the moderation roles + moderation_members = [MockMember(roles=[MockRole(id=role)]) for role in MODERATION_ROLES] + members = [MockMember(), *moderation_members] channel = MockVoiceChannel(members=members) await self.cog._force_voice_sync(channel) for member in members: - if any(role.id in MODERATION_ROLES for role in member.roles): + if member in moderation_members: member.move_to.assert_not_called() else: self.assertEqual(member.move_to.call_count, 2) @@ -213,14 +214,15 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): """Test to ensure kick function can remove all members from a voice channel.""" await self.cog._async_init() - members = [MockMember() for _ in range(10)] - members.extend([MockMember(roles=[MockRole(id=role)]) for role in MODERATION_ROLES]) + # Create a regular member, and one member for each of the moderation roles + moderation_members = [MockMember(roles=[MockRole(id=role)]) for role in MODERATION_ROLES] + members = [MockMember(), *moderation_members] channel = MockVoiceChannel(members=members) await self.cog._kick_voice_members(channel) for member in members: - if any(role.id in MODERATION_ROLES for role in member.roles): + if member in moderation_members: member.move_to.assert_not_called() else: self.assertEqual((None,), member.move_to.call_args_list[0].args) @@ -233,19 +235,15 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): Returns the list of erroneous members, as well as a list of regular and erroneous members combined, in that order. """ - erroneous_members = [MockMember(move_to=Mock(side_effect=Exception())) for _ in range(5)] + erroneous_member = MockMember(move_to=Mock(side_effect=Exception())) + members = [MockMember(), erroneous_member] - members = [] - for i in range(5): - members.append(MockMember()) - members.append(erroneous_members[i]) - - return erroneous_members, members + return erroneous_member, members async def test_kick_move_to_error(self): """Test to ensure move_to gets called on all members during kick, even if some fail.""" await self.cog._async_init() - failing_members, members = self.create_erroneous_members() + _, members = self.create_erroneous_members() await self.cog._kick_voice_members(MockVoiceChannel(members=members)) for member in members: @@ -254,11 +252,11 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): async def test_sync_move_to_error(self): """Test to ensure move_to gets called on all members during sync, even if some fail.""" await self.cog._async_init() - failing_members, members = self.create_erroneous_members() + failing_member, members = self.create_erroneous_members() await self.cog._force_voice_sync(MockVoiceChannel(members=members)) for member in members: - self.assertEqual(member.move_to.call_count, 1 if member in failing_members else 2) + self.assertEqual(member.move_to.call_count, 1 if member == failing_member else 2) @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) -- cgit v1.2.3 From da694fde0f78813a2787f371b8da84c39b72bcd9 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 24 Apr 2021 01:10:13 +0300 Subject: Uses Async Asserts Where Possible Signed-off-by: Hassan Abouelela --- tests/bot/exts/moderation/test_silence.py | 42 +++++++++++++++---------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 459048f68..ce76dc945 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -205,10 +205,10 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): overwrites = { channel.guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) } - channel.guild.create_voice_channel.assert_called_once_with("mute-temp", overwrites=overwrites) + channel.guild.create_voice_channel.assert_awaited_once_with("mute-temp", overwrites=overwrites) # Check bot deleted channel - new_channel.delete.assert_called_once() + new_channel.delete.assert_awaited_once() async def test_voice_kick(self): """Test to ensure kick function can remove all members from a voice channel.""" @@ -235,7 +235,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): Returns the list of erroneous members, as well as a list of regular and erroneous members combined, in that order. """ - erroneous_member = MockMember(move_to=Mock(side_effect=Exception())) + erroneous_member = MockMember(move_to=AsyncMock(side_effect=Exception())) members = [MockMember(), erroneous_member] return erroneous_member, members @@ -247,7 +247,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): await self.cog._kick_voice_members(MockVoiceChannel(members=members)) for member in members: - member.move_to.assert_called_once() + member.move_to.assert_awaited_once() async def test_sync_move_to_error(self): """Test to ensure move_to gets called on all members during sync, even if some fail.""" @@ -399,7 +399,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockVoiceChannel() await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=False) - sync.assert_called_once_with(self.cog, channel) + sync.assert_awaited_once_with(self.cog, channel) kick.assert_not_called() @voice_sync_helper @@ -408,7 +408,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockVoiceChannel() await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=True) - kick.assert_called_once_with(channel) + kick.assert_awaited_once_with(channel) sync.assert_not_called() @voice_sync_helper @@ -510,7 +510,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): """Channel's previous overwrites were cached.""" overwrite_json = '{"send_messages": true, "add_reactions": false}' await self.cog._set_silence_overwrites(self.text_channel) - self.cog.previous_overwrites.set.assert_called_once_with(self.text_channel.id, overwrite_json) + self.cog.previous_overwrites.set.assert_awaited_once_with(self.text_channel.id, overwrite_json) @autospec(silence, "datetime") async def test_cached_unsilence_time(self, datetime_mock): @@ -597,7 +597,7 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.unsilence.callback(self.cog, ctx, channel=target) call_args = (message, ctx.channel, target or ctx.channel) - send_message.assert_called_once_with(*call_args, alert_target=was_unsilenced) + send_message.assert_awaited_once_with(*call_args, alert_target=was_unsilenced) async def test_skipped_already_unsilenced(self): """Permissions were not set and `False` was returned for an already unsilenced channel.""" @@ -731,11 +731,11 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence_wrapper(channel, context) if context is None: - send_message.assert_called_once_with(message, channel, channel, alert_target=True) + send_message.assert_awaited_once_with(message, channel, channel, alert_target=True) else: - send_message.assert_called_once_with(message, context.channel, channel, alert_target=True) + send_message.assert_awaited_once_with(message, context.channel, channel, alert_target=True) - channel.set_permissions.assert_called_once_with(role, overwrite=overwrites) + channel.set_permissions.assert_awaited_once_with(role, overwrite=overwrites) if channel != ctx.channel: ctx.channel.send.assert_not_called() @@ -759,7 +759,7 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): message = "Test basic message." await self.cog.send_message(message, *self.text_channels, alert_target=False) - self.text_channels[0].send.assert_called_once_with(message) + self.text_channels[0].send.assert_awaited_once_with(message) self.text_channels[1].send.assert_not_called() async def test_send_to_multiple_channels(self): @@ -767,8 +767,8 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): message = "Test basic message." await self.cog.send_message(message, *self.text_channels, alert_target=True) - self.text_channels[0].send.assert_called_once_with(message) - self.text_channels[1].send.assert_called_once_with(message) + self.text_channels[0].send.assert_awaited_once_with(message) + self.text_channels[1].send.assert_awaited_once_with(message) async def test_duration_replacement(self): """Tests that the channel name was set correctly for one target channel.""" @@ -776,7 +776,7 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): await self.cog.send_message(message, *self.text_channels, alert_target=False) updated_message = message.replace("current channel", self.text_channels[0].mention) - self.text_channels[0].send.assert_called_once_with(updated_message) + self.text_channels[0].send.assert_awaited_once_with(updated_message) self.text_channels[1].send.assert_not_called() async def test_name_replacement_multiple_channels(self): @@ -785,14 +785,14 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): await self.cog.send_message(message, *self.text_channels, alert_target=True) updated_message = message.replace("current channel", self.text_channels[0].mention) - self.text_channels[0].send.assert_called_once_with(updated_message) - self.text_channels[1].send.assert_called_once_with(message) + self.text_channels[0].send.assert_awaited_once_with(updated_message) + self.text_channels[1].send.assert_awaited_once_with(message) async def test_silence_voice(self): """Tests that the correct message was sent when a voice channel is muted without alerting.""" message = "This should show up just here." await self.cog.send_message(message, self.text_channels[0], self.voice_channel, alert_target=False) - self.text_channels[0].send.assert_called_once_with(message) + self.text_channels[0].send.assert_awaited_once_with(message) self.text_channels[1].send.assert_not_called() async def test_silence_voice_alert(self): @@ -804,8 +804,8 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): await self.cog.send_message(message, self.text_channels[0], self.voice_channel, alert_target=True) updated_message = message.replace("current channel", self.voice_channel.mention) - self.text_channels[0].send.assert_called_once_with(updated_message) - self.text_channels[1].send.assert_called_once_with(updated_message) + self.text_channels[0].send.assert_awaited_once_with(updated_message) + self.text_channels[1].send.assert_awaited_once_with(updated_message) mock_voice_channels.get.assert_called_once_with(self.voice_channel.id) @@ -818,7 +818,7 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): await self.cog.send_message(message, self.text_channels[1], self.voice_channel, alert_target=True) updated_message = message.replace("current channel", self.voice_channel.mention) - self.text_channels[1].send.assert_called_once_with(updated_message) + self.text_channels[1].send.assert_awaited_once_with(updated_message) mock_voice_channels.get.assert_called_once_with(self.voice_channel.id) self.bot.get_channel.assert_called_once_with(self.text_channels[1].id) -- cgit v1.2.3 From db5b0751fb9531525e12e6d421aa7d0772b3054a Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 4 May 2021 04:37:49 +0300 Subject: Copy Existing Text Channel Cache Tests For Voice Duplicates existing silence and unsilence cache tests for voice channels. Signed-off-by: Hassan Abouelela --- tests/bot/exts/moderation/test_silence.py | 179 ++++++++++++++++-------------- 1 file changed, 97 insertions(+), 82 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index ce76dc945..347471c13 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -362,11 +362,11 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): asyncio.run(self.cog._async_init()) # Populate instance attributes. self.text_channel = MockTextChannel() - self.text_overwrite = PermissionOverwrite(stream=True, send_messages=True, add_reactions=False) + self.text_overwrite = PermissionOverwrite(send_messages=True, add_reactions=False) self.text_channel.overwrites_for.return_value = self.text_overwrite self.voice_channel = MockVoiceChannel() - self.voice_overwrite = PermissionOverwrite(speak=True) + self.voice_overwrite = PermissionOverwrite(connect=True, speak=True) self.voice_channel.overwrites_for.return_value = self.voice_overwrite async def test_sent_correct_message(self): @@ -474,8 +474,8 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): overwrite=self.voice_overwrite ) - async def test_preserved_other_overwrites(self): - """Channel's other unrelated overwrites were not changed.""" + async def test_preserved_other_overwrites_text(self): + """Channel's other unrelated overwrites were not changed for a text channel mute.""" prev_overwrite_dict = dict(self.text_overwrite) await self.cog._set_silence_overwrites(self.text_channel) new_overwrite_dict = dict(self.text_overwrite) @@ -488,6 +488,20 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) + async def test_preserved_other_overwrites_voice(self): + """Channel's other unrelated overwrites were not changed for a voice channel mute.""" + prev_overwrite_dict = dict(self.voice_overwrite) + await self.cog._set_silence_overwrites(self.voice_channel) + new_overwrite_dict = dict(self.voice_overwrite) + + # Remove 'connect' & 'speak' keys because they were changed by the method. + del prev_overwrite_dict['connect'] + del prev_overwrite_dict['speak'] + del new_overwrite_dict['connect'] + del new_overwrite_dict['speak'] + + self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) + async def test_temp_not_added_to_notifier(self): """Channel was not added to notifier if a duration was set for the silence.""" with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): @@ -568,9 +582,13 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): self.cog.scheduler.__contains__.return_value = True overwrites_cache.get.return_value = '{"send_messages": true, "add_reactions": false}' - self.channel = MockTextChannel() - self.overwrite = PermissionOverwrite(stream=True, send_messages=False, add_reactions=False) - self.channel.overwrites_for.return_value = self.overwrite + self.text_channel = MockTextChannel() + self.text_overwrite = PermissionOverwrite(send_messages=False, add_reactions=False) + self.text_channel.overwrites_for.return_value = self.text_overwrite + + self.voice_channel = MockVoiceChannel() + self.voice_overwrite = PermissionOverwrite(connect=True, speak=True) + self.voice_channel.overwrites_for.return_value = self.voice_overwrite async def test_sent_correct_message(self): """Appropriate failure/success message was sent by the command.""" @@ -578,7 +596,7 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): test_cases = ( (True, silence.MSG_UNSILENCE_SUCCESS, unsilenced_overwrite), (False, silence.MSG_UNSILENCE_FAIL, unsilenced_overwrite), - (False, silence.MSG_UNSILENCE_MANUAL, self.overwrite), + (False, silence.MSG_UNSILENCE_MANUAL, self.text_overwrite), (False, silence.MSG_UNSILENCE_MANUAL, PermissionOverwrite(send_messages=False)), (False, silence.MSG_UNSILENCE_MANUAL, PermissionOverwrite(add_reactions=False)), ) @@ -608,35 +626,60 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): self.assertFalse(await self.cog._unsilence(channel)) channel.set_permissions.assert_not_called() - async def test_restored_overwrites(self): - """Channel's `send_message` and `add_reactions` overwrites were restored.""" - await self.cog._unsilence(self.channel) - self.channel.set_permissions.assert_awaited_once_with( + async def test_restored_overwrites_text(self): + """Text channel's `send_message` and `add_reactions` overwrites were restored.""" + await self.cog._unsilence(self.text_channel) + self.text_channel.set_permissions.assert_awaited_once_with( self.cog._everyone_role, - overwrite=self.overwrite, + overwrite=self.text_overwrite, + ) + + # Recall that these values are determined by the fixture. + self.assertTrue(self.text_overwrite.send_messages) + self.assertFalse(self.text_overwrite.add_reactions) + + async def test_restored_overwrites_voice(self): + """Voice channel's `connect` and `speak` overwrites were restored.""" + await self.cog._unsilence(self.voice_channel) + self.voice_channel.set_permissions.assert_awaited_once_with( + self.cog._verified_voice_role, + overwrite=self.voice_overwrite, ) # Recall that these values are determined by the fixture. - self.assertTrue(self.overwrite.send_messages) - self.assertFalse(self.overwrite.add_reactions) + self.assertTrue(self.voice_overwrite.connect) + self.assertTrue(self.voice_overwrite.speak) - async def test_cache_miss_used_default_overwrites(self): - """Both overwrites were set to None due previous values not being found in the cache.""" + async def test_cache_miss_used_default_overwrites_text(self): + """Text overwrites were set to None due previous values not being found in the cache.""" self.cog.previous_overwrites.get.return_value = None - await self.cog._unsilence(self.channel) - self.channel.set_permissions.assert_awaited_once_with( + await self.cog._unsilence(self.text_channel) + self.text_channel.set_permissions.assert_awaited_once_with( self.cog._everyone_role, - overwrite=self.overwrite, + overwrite=self.text_overwrite, ) - self.assertIsNone(self.overwrite.send_messages) - self.assertIsNone(self.overwrite.add_reactions) + self.assertIsNone(self.text_overwrite.send_messages) + self.assertIsNone(self.text_overwrite.add_reactions) + + async def test_cache_miss_used_default_overwrites_voice(self): + """Voice overwrites were set to None due previous values not being found in the cache.""" + self.cog.previous_overwrites.get.return_value = None + + await self.cog._unsilence(self.voice_channel) + self.voice_channel.set_permissions.assert_awaited_once_with( + self.cog._verified_voice_role, + overwrite=self.voice_overwrite, + ) + + self.assertIsNone(self.voice_overwrite.connect) + self.assertIsNone(self.voice_overwrite.speak) async def test_cache_miss_sent_mod_alert_text(self): """A message was sent to the mod alerts channel upon muting a text channel.""" self.cog.previous_overwrites.get.return_value = None - await self.cog._unsilence(self.channel) + await self.cog._unsilence(self.text_channel) self.cog._mod_alerts_channel.send.assert_awaited_once() async def test_cache_miss_sent_mod_alert_voice(self): @@ -647,33 +690,33 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): async def test_removed_notifier(self): """Channel was removed from `notifier`.""" - await self.cog._unsilence(self.channel) - self.cog.notifier.remove_channel.assert_called_once_with(self.channel) + await self.cog._unsilence(self.text_channel) + self.cog.notifier.remove_channel.assert_called_once_with(self.text_channel) async def test_deleted_cached_overwrite(self): """Channel was deleted from the overwrites cache.""" - await self.cog._unsilence(self.channel) - self.cog.previous_overwrites.delete.assert_awaited_once_with(self.channel.id) + await self.cog._unsilence(self.text_channel) + self.cog.previous_overwrites.delete.assert_awaited_once_with(self.text_channel.id) async def test_deleted_cached_time(self): """Channel was deleted from the timestamp cache.""" - await self.cog._unsilence(self.channel) - self.cog.unsilence_timestamps.delete.assert_awaited_once_with(self.channel.id) + await self.cog._unsilence(self.text_channel) + self.cog.unsilence_timestamps.delete.assert_awaited_once_with(self.text_channel.id) async def test_cancelled_task(self): """The scheduled unsilence task should be cancelled.""" - await self.cog._unsilence(self.channel) - self.cog.scheduler.cancel.assert_called_once_with(self.channel.id) + await self.cog._unsilence(self.text_channel) + self.cog.scheduler.cancel.assert_called_once_with(self.text_channel.id) - async def test_preserved_other_overwrites(self): - """Channel's other unrelated overwrites were not changed, including cache misses.""" + async def test_preserved_other_overwrites_text(self): + """Text channel's other unrelated overwrites were not changed, including cache misses.""" for overwrite_json in ('{"send_messages": true, "add_reactions": null}', None): with self.subTest(overwrite_json=overwrite_json): self.cog.previous_overwrites.get.return_value = overwrite_json - prev_overwrite_dict = dict(self.overwrite) - await self.cog._unsilence(self.channel) - new_overwrite_dict = dict(self.overwrite) + prev_overwrite_dict = dict(self.text_overwrite) + await self.cog._unsilence(self.text_channel) + new_overwrite_dict = dict(self.text_overwrite) # Remove these keys because they were modified by the unsilence. del prev_overwrite_dict['send_messages'] @@ -683,6 +726,24 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) + async def test_preserved_other_overwrites_voice(self): + """Voice channel's other unrelated overwrites were not changed, including cache misses.""" + for overwrite_json in ('{"connect": true, "speak": true}', None): + with self.subTest(overwrite_json=overwrite_json): + self.cog.previous_overwrites.get.return_value = overwrite_json + + prev_overwrite_dict = dict(self.voice_overwrite) + await self.cog._unsilence(self.voice_channel) + new_overwrite_dict = dict(self.voice_overwrite) + + # Remove these keys because they were modified by the unsilence. + del prev_overwrite_dict['connect'] + del prev_overwrite_dict['speak'] + del new_overwrite_dict['connect'] + del new_overwrite_dict['speak'] + + self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) + async def test_unsilence_role(self): """Tests unsilence_wrapper applies permission to the correct role.""" test_cases = ( @@ -695,52 +756,6 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence_wrapper(channel, MockContext()) channel.overwrites_for.assert_called_with(role) - @mock.patch.object(silence.Silence, "_force_voice_sync") - @mock.patch.object(silence.Silence, "send_message") - async def test_correct_overwrites(self, send_message, _): - """Tests the overwrites returned by the _unsilence_wrapper are correct for voice and text channels.""" - ctx = MockContext() - - text_channel = MockTextChannel() - text_role = self.cog.bot.get_guild(Guild.id).default_role - - voice_channel = MockVoiceChannel() - voice_role = self.cog.bot.get_guild(Guild.id).get_role(Roles.voice_verified) - - async def reset(): - await text_channel.set_permissions(text_role, PermissionOverwrite(send_messages=False, add_reactions=False)) - await voice_channel.set_permissions(voice_role, PermissionOverwrite(speak=False, connect=False)) - - text_channel.reset_mock() - voice_channel.reset_mock() - send_message.reset_mock() - await reset() - - default_text_overwrites = text_channel.overwrites_for(text_role) - default_voice_overwrites = voice_channel.overwrites_for(voice_role) - - test_cases = ( - (ctx, text_channel, text_role, default_text_overwrites, silence.MSG_UNSILENCE_SUCCESS), - (ctx, voice_channel, voice_role, default_voice_overwrites, silence.MSG_UNSILENCE_SUCCESS), - (ctx, ctx.channel, text_role, ctx.channel.overwrites_for(text_role), silence.MSG_UNSILENCE_SUCCESS), - (None, text_channel, text_role, default_text_overwrites, silence.MSG_UNSILENCE_SUCCESS), - ) - - for context, channel, role, overwrites, message in test_cases: - with self.subTest(ctx=context, channel=channel): - await self.cog._unsilence_wrapper(channel, context) - - if context is None: - send_message.assert_awaited_once_with(message, channel, channel, alert_target=True) - else: - send_message.assert_awaited_once_with(message, context.channel, channel, alert_target=True) - - channel.set_permissions.assert_awaited_once_with(role, overwrite=overwrites) - if channel != ctx.channel: - ctx.channel.send.assert_not_called() - - await reset() - class SendMessageTests(unittest.IsolatedAsyncioTestCase): """Unittests for the send message helper function.""" -- cgit v1.2.3 From bac68d1d584b398f2bc5cc7a8f3df39ed48174ae Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 4 May 2021 04:47:15 +0300 Subject: Adds Voice Test Cases To Already Silenced Test Signed-off-by: Hassan Abouelela --- tests/bot/exts/moderation/test_silence.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 347471c13..0d135698e 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -432,15 +432,17 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_skipped_already_silenced(self): """Permissions were not set and `False` was returned for an already silenced channel.""" subtests = ( - (False, PermissionOverwrite(send_messages=False, add_reactions=False)), - (True, PermissionOverwrite(send_messages=True, add_reactions=True)), - (True, PermissionOverwrite(send_messages=False, add_reactions=False)), + (False, MockTextChannel(), PermissionOverwrite(send_messages=False, add_reactions=False)), + (True, MockTextChannel(), PermissionOverwrite(send_messages=True, add_reactions=True)), + (True, MockTextChannel(), PermissionOverwrite(send_messages=False, add_reactions=False)), + (False, MockVoiceChannel(), PermissionOverwrite(connect=False, speak=False)), + (True, MockVoiceChannel(), PermissionOverwrite(connect=True, speak=True)), + (True, MockVoiceChannel(), PermissionOverwrite(connect=False, speak=False)), ) - for contains, overwrite in subtests: - with self.subTest(contains=contains, overwrite=overwrite): + for contains, channel, overwrite in subtests: + with self.subTest(contains=contains, is_text=isinstance(channel, MockTextChannel), overwrite=overwrite): self.cog.scheduler.__contains__.return_value = contains - channel = MockTextChannel() channel.overwrites_for.return_value = overwrite self.assertFalse(await self.cog._set_silence_overwrites(channel)) -- cgit v1.2.3 From 4bf9a7a545e2ffb507fbb379df1695755a2eea1b Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 4 May 2021 05:09:04 +0300 Subject: Adds Missing Voice Version Of Tests Signed-off-by: Hassan Abouelela --- tests/bot/exts/moderation/test_silence.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 0d135698e..729b28412 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -623,10 +623,11 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): """Permissions were not set and `False` was returned for an already unsilenced channel.""" self.cog.scheduler.__contains__.return_value = False self.cog.previous_overwrites.get.return_value = None - channel = MockTextChannel() - self.assertFalse(await self.cog._unsilence(channel)) - channel.set_permissions.assert_not_called() + for channel in (MockVoiceChannel(), MockTextChannel()): + with self.subTest(channel=channel): + self.assertFalse(await self.cog._unsilence(channel)) + channel.set_permissions.assert_not_called() async def test_restored_overwrites_text(self): """Text channel's `send_message` and `add_reactions` overwrites were restored.""" -- cgit v1.2.3 From 2722352236bcad411bd6417e7009e43625a7da9a Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sun, 9 May 2021 00:32:13 +0300 Subject: Uses Itertools Product To Reduce Nesting Uses itertools.product to eliminate some nested for loops in tests. Signed-off-by: Hassan Abouelela --- tests/bot/exts/moderation/test_silence.py | 50 ++++++++++++++++--------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 729b28412..de7230ae5 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -1,4 +1,5 @@ import asyncio +import itertools import unittest from datetime import datetime, timezone from typing import List, Tuple @@ -379,19 +380,20 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): (5, silence.MSG_SILENCE_FAIL, False,), ) - for duration, message, was_silenced in test_cases: + targets = (MockTextChannel(), MockVoiceChannel(), None) + + for (duration, message, was_silenced), target in itertools.product(test_cases, targets): with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=was_silenced): - for target in [MockTextChannel(), MockVoiceChannel(), None]: - with self.subTest(was_silenced=was_silenced, target=target, message=message): - with mock.patch.object(self.cog, "send_message") as send_message: - ctx = MockContext() - await self.cog.silence.callback(self.cog, ctx, duration, target) - send_message.assert_called_once_with( - message, - ctx.channel, - target or ctx.channel, - alert_target=was_silenced - ) + with self.subTest(was_silenced=was_silenced, target=target, message=message): + with mock.patch.object(self.cog, "send_message") as send_message: + ctx = MockContext() + await self.cog.silence.callback(self.cog, ctx, duration, target) + send_message.assert_called_once_with( + message, + ctx.channel, + target or ctx.channel, + alert_target=was_silenced + ) @voice_sync_helper async def test_sync_called(self, ctx, sync, kick): @@ -603,21 +605,21 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): (False, silence.MSG_UNSILENCE_MANUAL, PermissionOverwrite(add_reactions=False)), ) - for was_unsilenced, message, overwrite in test_cases: - ctx = MockContext() + targets = (None, MockTextChannel()) - for target in [None, MockTextChannel()]: - ctx.channel.overwrites_for.return_value = overwrite - if target: - target.overwrites_for.return_value = overwrite + for (was_unsilenced, message, overwrite), target in itertools.product(test_cases, targets): + ctx = MockContext() + ctx.channel.overwrites_for.return_value = overwrite + if target: + target.overwrites_for.return_value = overwrite - with mock.patch.object(self.cog, "_unsilence", return_value=was_unsilenced): - with mock.patch.object(self.cog, "send_message") as send_message: - with self.subTest(was_unsilenced=was_unsilenced, overwrite=overwrite, target=target): - await self.cog.unsilence.callback(self.cog, ctx, channel=target) + with mock.patch.object(self.cog, "_unsilence", return_value=was_unsilenced): + with mock.patch.object(self.cog, "send_message") as send_message: + with self.subTest(was_unsilenced=was_unsilenced, overwrite=overwrite, target=target): + await self.cog.unsilence.callback(self.cog, ctx, channel=target) - call_args = (message, ctx.channel, target or ctx.channel) - send_message.assert_awaited_once_with(*call_args, alert_target=was_unsilenced) + call_args = (message, ctx.channel, target or ctx.channel) + send_message.assert_awaited_once_with(*call_args, alert_target=was_unsilenced) async def test_skipped_already_unsilenced(self): """Permissions were not set and `False` was returned for an already unsilenced channel.""" -- cgit v1.2.3 From c896414b1669ce63e747df292b96e0a23595140b Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Mon, 10 May 2021 19:54:38 +0300 Subject: Fixes Function Signature Formatting Signed-off-by: Hassan Abouelela --- bot/exts/moderation/silence.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 616dfbefb..a3174bc5d 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -128,7 +128,8 @@ class Silence(commands.Cog): message: str, source_channel: TextChannel, target_channel: TextOrVoiceChannel, - *, alert_target: bool = False + *, + alert_target: bool = False ) -> None: """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`.""" # Reply to invocation channel @@ -154,7 +155,8 @@ class Silence(commands.Cog): ctx: Context, duration: HushDurationConverter = 10, channel: TextOrVoiceChannel = None, - *, kick: bool = False + *, + kick: bool = False ) -> None: """ Silence the current channel for `duration` minutes or `forever`. -- cgit v1.2.3 From d9ff75759cfd3728ba95c89c5aa2dbeae4b1f339 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Mon, 10 May 2021 20:10:17 +0300 Subject: Fixes Logging Statement Changes logging statement levels and messages to correctly express intent. Signed-off-by: Hassan Abouelela --- bot/exts/moderation/silence.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index a3174bc5d..b2b781673 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -274,7 +274,7 @@ class Silence(commands.Cog): guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) } afk_channel = await guild.create_voice_channel("mute-temp", overwrites=overwrites) - log.info(f"Failed to get afk-channel, created temporary channel #{afk_channel} ({afk_channel.id})") + log.info(f"Failed to get afk-channel, created #{afk_channel} ({afk_channel.id})") return afk_channel @@ -290,7 +290,7 @@ class Silence(commands.Cog): try: await member.move_to(None, reason="Kicking member from voice channel.") - log.debug(f"Kicked {member.name} from voice channel.") + log.trace(f"Kicked {member.name} from voice channel.") except Exception as e: log.debug(f"Failed to move {member.name}. Reason: {e}") continue @@ -316,10 +316,10 @@ class Silence(commands.Cog): try: await member.move_to(afk_channel, reason="Muting VC member.") - log.debug(f"Moved {member.name} to afk channel.") + log.trace(f"Moved {member.name} to afk channel.") await member.move_to(channel, reason="Muting VC member.") - log.debug(f"Moved {member.name} to original voice channel.") + log.trace(f"Moved {member.name} to original voice channel.") except Exception as e: log.debug(f"Failed to move {member.name}. Reason: {e}") continue -- cgit v1.2.3 From 0253f8f6ac0d0cd4878beb320f55d34300bea716 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Mon, 10 May 2021 20:18:35 +0300 Subject: Restructures Silence Cog Restructures silence cog helper methods to group relation functions in a more logical manner. Signed-off-by: Hassan Abouelela --- bot/exts/moderation/silence.py | 156 ++++++++++++++++++++--------------------- 1 file changed, 78 insertions(+), 78 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index b2b781673..289072e8b 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -196,6 +196,41 @@ class Silence(commands.Cog): formatted_message = MSG_SILENCE_SUCCESS.format(duration=duration) await self.send_message(formatted_message, ctx.channel, channel, alert_target=True) + async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, *, kick: bool = False) -> bool: + """Set silence permission overwrites for `channel` and return True if successful.""" + # Get the original channel overwrites + if isinstance(channel, TextChannel): + role = self._everyone_role + overwrite = channel.overwrites_for(role) + prev_overwrites = dict(send_messages=overwrite.send_messages, add_reactions=overwrite.add_reactions) + + else: + role = self._verified_voice_role + overwrite = channel.overwrites_for(role) + prev_overwrites = dict(speak=overwrite.speak) + if kick: + prev_overwrites.update(connect=overwrite.connect) + + # Stop if channel was already silenced + if channel.id in self.scheduler or all(val is False for val in prev_overwrites.values()): + return False + + # Set new permissions, store + overwrite.update(**dict.fromkeys(prev_overwrites, False)) + await channel.set_permissions(role, overwrite=overwrite) + await self.previous_overwrites.set(channel.id, json.dumps(prev_overwrites)) + + return True + + async def _schedule_unsilence(self, ctx: Context, channel: TextOrVoiceChannel, duration: Optional[int]) -> None: + """Schedule `ctx.channel` to be unsilenced if `duration` is not None.""" + if duration is None: + await self.unsilence_timestamps.set(channel.id, -1) + else: + self.scheduler.schedule_later(duration * 60, channel.id, ctx.invoke(self.unsilence, channel=channel)) + unsilence_time = datetime.now(tz=timezone.utc) + timedelta(minutes=duration) + await self.unsilence_timestamps.set(channel.id, unsilence_time.timestamp()) + @commands.command(aliases=("unhush",)) async def unsilence(self, ctx: Context, *, channel: TextOrVoiceChannel = None) -> None: """ @@ -238,29 +273,58 @@ class Silence(commands.Cog): else: await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, alert_target=True) - async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, *, kick: bool = False) -> bool: - """Set silence permission overwrites for `channel` and return True if successful.""" - # Get the original channel overwrites + async def _unsilence(self, channel: TextOrVoiceChannel) -> bool: + """ + Unsilence `channel`. + + If `channel` has a silence task scheduled or has its previous overwrites cached, unsilence + it, cancel the task, and remove it from the notifier. Notify admins if it has a task but + not cached overwrites. + + Return `True` if channel permissions were changed, `False` otherwise. + """ + # Get stored overwrites, and return if channel is unsilenced + prev_overwrites = await self.previous_overwrites.get(channel.id) + if channel.id not in self.scheduler and prev_overwrites is None: + log.info(f"Tried to unsilence channel #{channel} ({channel.id}) but the channel was not silenced.") + return False + + # Select the role based on channel type, and get current overwrites if isinstance(channel, TextChannel): role = self._everyone_role overwrite = channel.overwrites_for(role) - prev_overwrites = dict(send_messages=overwrite.send_messages, add_reactions=overwrite.add_reactions) - + permissions = "`Send Messages` and `Add Reactions`" else: role = self._verified_voice_role overwrite = channel.overwrites_for(role) - prev_overwrites = dict(speak=overwrite.speak) - if kick: - prev_overwrites.update(connect=overwrite.connect) + permissions = "`Speak` and `Connect`" - # Stop if channel was already silenced - if channel.id in self.scheduler or all(val is False for val in prev_overwrites.values()): - return False + # Check if old overwrites were not stored + if prev_overwrites is None: + log.info(f"Missing previous overwrites for #{channel} ({channel.id}); defaulting to None.") + overwrite.update(send_messages=None, add_reactions=None, speak=None, connect=None) + else: + overwrite.update(**json.loads(prev_overwrites)) - # Set new permissions, store - overwrite.update(**dict.fromkeys(prev_overwrites, False)) + # Update Permissions await channel.set_permissions(role, overwrite=overwrite) - await self.previous_overwrites.set(channel.id, json.dumps(prev_overwrites)) + if isinstance(channel, VoiceChannel): + await self._force_voice_sync(channel) + + log.info(f"Unsilenced channel #{channel} ({channel.id}).") + + self.scheduler.cancel(channel.id) + self.notifier.remove_channel(channel) + await self.previous_overwrites.delete(channel.id) + await self.unsilence_timestamps.delete(channel.id) + + # Alert Admin team if old overwrites were not available + if prev_overwrites is None: + await self._mod_alerts_channel.send( + f"<@&{constants.Roles.admins}> Restored overwrites with default values after unsilencing " + f"{channel.mention}. Please check that the {permissions} " + f"overwrites for {role.mention} are at their desired values." + ) return True @@ -329,70 +393,6 @@ class Silence(commands.Cog): if delete_channel: await afk_channel.delete(reason="Deleting temporary mute channel.") - async def _schedule_unsilence(self, ctx: Context, channel: TextOrVoiceChannel, duration: Optional[int]) -> None: - """Schedule `ctx.channel` to be unsilenced if `duration` is not None.""" - if duration is None: - await self.unsilence_timestamps.set(channel.id, -1) - else: - self.scheduler.schedule_later(duration * 60, channel.id, ctx.invoke(self.unsilence, channel=channel)) - unsilence_time = datetime.now(tz=timezone.utc) + timedelta(minutes=duration) - await self.unsilence_timestamps.set(channel.id, unsilence_time.timestamp()) - - async def _unsilence(self, channel: TextOrVoiceChannel) -> bool: - """ - Unsilence `channel`. - - If `channel` has a silence task scheduled or has its previous overwrites cached, unsilence - it, cancel the task, and remove it from the notifier. Notify admins if it has a task but - not cached overwrites. - - Return `True` if channel permissions were changed, `False` otherwise. - """ - # Get stored overwrites, and return if channel is unsilenced - prev_overwrites = await self.previous_overwrites.get(channel.id) - if channel.id not in self.scheduler and prev_overwrites is None: - log.info(f"Tried to unsilence channel #{channel} ({channel.id}) but the channel was not silenced.") - return False - - # Select the role based on channel type, and get current overwrites - if isinstance(channel, TextChannel): - role = self._everyone_role - overwrite = channel.overwrites_for(role) - permissions = "`Send Messages` and `Add Reactions`" - else: - role = self._verified_voice_role - overwrite = channel.overwrites_for(role) - permissions = "`Speak` and `Connect`" - - # Check if old overwrites were not stored - if prev_overwrites is None: - log.info(f"Missing previous overwrites for #{channel} ({channel.id}); defaulting to None.") - overwrite.update(send_messages=None, add_reactions=None, speak=None, connect=None) - else: - overwrite.update(**json.loads(prev_overwrites)) - - # Update Permissions - await channel.set_permissions(role, overwrite=overwrite) - if isinstance(channel, VoiceChannel): - await self._force_voice_sync(channel) - - log.info(f"Unsilenced channel #{channel} ({channel.id}).") - - self.scheduler.cancel(channel.id) - self.notifier.remove_channel(channel) - await self.previous_overwrites.delete(channel.id) - await self.unsilence_timestamps.delete(channel.id) - - # Alert Admin team if old overwrites were not available - if prev_overwrites is None: - await self._mod_alerts_channel.send( - f"<@&{constants.Roles.admins}> Restored overwrites with default values after unsilencing " - f"{channel.mention}. Please check that the {permissions} " - f"overwrites for {role.mention} are at their desired values." - ) - - return True - async def _reschedule(self) -> None: """Reschedule unsilencing of active silences and add permanent ones to the notifier.""" for channel_id, timestamp in await self.unsilence_timestamps.items(): -- cgit v1.2.3 From 52bfbf314c4a0de8b6de434718589d484a6da95c Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Mon, 10 May 2021 20:23:50 +0300 Subject: Rename `Manual` Variable To Clarify Intentions Signed-off-by: Hassan Abouelela --- bot/exts/moderation/silence.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 289072e8b..2012d75d9 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -259,13 +259,13 @@ class Silence(commands.Cog): if not await self._unsilence(channel): if isinstance(channel, VoiceChannel): overwrite = channel.overwrites_for(self._verified_voice_role) - manual = overwrite.speak is False + has_channel_overwrites = overwrite.speak is False else: overwrite = channel.overwrites_for(self._everyone_role) - manual = overwrite.send_messages is False or overwrite.add_reactions is False + has_channel_overwrites = overwrite.send_messages is False or overwrite.add_reactions is False # Send fail message to muted channel or voice chat channel, and invocation channel - if manual: + if has_channel_overwrites: await self.send_message(MSG_UNSILENCE_MANUAL, msg_channel, channel, alert_target=False) else: await self.send_message(MSG_UNSILENCE_FAIL, msg_channel, channel, alert_target=False) -- cgit v1.2.3 From 2f772082f288671354b6bbd71d9a9caad6fe87af Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Wed, 12 May 2021 00:03:26 +0300 Subject: Updates Silence To Use `.format` Uses `.format` to create silence and unsilence messages instead of `.replace`. Signed-off-by: Hassan Abouelela --- bot/exts/moderation/silence.py | 20 +++++++++++--------- tests/bot/exts/moderation/test_silence.py | 19 +++++++++---------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 2012d75d9..e5c96e76f 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -19,17 +19,17 @@ log = logging.getLogger(__name__) LOCK_NAMESPACE = "silence" -MSG_SILENCE_FAIL = f"{constants.Emojis.cross_mark} current channel is already silenced." -MSG_SILENCE_PERMANENT = f"{constants.Emojis.check_mark} silenced current channel indefinitely." -MSG_SILENCE_SUCCESS = f"{constants.Emojis.check_mark} silenced current channel for {{duration}} minute(s)." +MSG_SILENCE_FAIL = f"{constants.Emojis.cross_mark} {{channel}} is already silenced." +MSG_SILENCE_PERMANENT = f"{constants.Emojis.check_mark} silenced {{channel}} indefinitely." +MSG_SILENCE_SUCCESS = f"{constants.Emojis.check_mark} silenced {{{{channel}}}} for {{duration}} minute(s)." -MSG_UNSILENCE_FAIL = f"{constants.Emojis.cross_mark} current channel was not silenced." +MSG_UNSILENCE_FAIL = f"{constants.Emojis.cross_mark} {{channel}} was not silenced." MSG_UNSILENCE_MANUAL = ( - f"{constants.Emojis.cross_mark} current channel was not unsilenced because the current overwrites were " + f"{constants.Emojis.cross_mark} {{channel}} was not unsilenced because the current overwrites were " f"set manually or the cache was prematurely cleared. " f"Please edit the overwrites manually to unsilence." ) -MSG_UNSILENCE_SUCCESS = f"{constants.Emojis.check_mark} unsilenced current channel." +MSG_UNSILENCE_SUCCESS = f"{constants.Emojis.check_mark} unsilenced {{channel}}." TextOrVoiceChannel = Union[TextChannel, VoiceChannel] @@ -135,7 +135,9 @@ class Silence(commands.Cog): # Reply to invocation channel source_reply = message if source_channel != target_channel: - source_reply = source_reply.replace("current channel", target_channel.mention) + source_reply = source_reply.format(channel=target_channel.mention) + else: + source_reply = source_reply.format(channel="current channel") await source_channel.send(source_reply) # Reply to target channel @@ -143,10 +145,10 @@ class Silence(commands.Cog): if isinstance(target_channel, VoiceChannel): voice_chat = self.bot.get_channel(VOICE_CHANNELS.get(target_channel.id)) if voice_chat and source_channel != voice_chat: - await voice_chat.send(message.replace("current channel", target_channel.mention)) + await voice_chat.send(message.format(channel=target_channel.mention)) elif source_channel != target_channel: - await target_channel.send(message) + await target_channel.send(message.format(channel="current channel")) @commands.command(aliases=("hush",)) @lock(LOCK_NAMESPACE, _select_lock_channel, raise_error=True) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index de7230ae5..af6dd5a37 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -792,21 +792,20 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): async def test_duration_replacement(self): """Tests that the channel name was set correctly for one target channel.""" - message = "Current. The following should be replaced: current channel." + message = "Current. The following should be replaced: {channel}." await self.cog.send_message(message, *self.text_channels, alert_target=False) - updated_message = message.replace("current channel", self.text_channels[0].mention) + updated_message = message.format(channel=self.text_channels[0].mention) self.text_channels[0].send.assert_awaited_once_with(updated_message) self.text_channels[1].send.assert_not_called() async def test_name_replacement_multiple_channels(self): """Tests that the channel name was set correctly for two channels.""" - message = "Current. The following should be replaced: current channel." + message = "Current. The following should be replaced: {channel}." await self.cog.send_message(message, *self.text_channels, alert_target=True) - updated_message = message.replace("current channel", self.text_channels[0].mention) - self.text_channels[0].send.assert_awaited_once_with(updated_message) - self.text_channels[1].send.assert_awaited_once_with(message) + self.text_channels[0].send.assert_awaited_once_with(message.format(channel=self.text_channels[0].mention)) + self.text_channels[1].send.assert_awaited_once_with(message.format(channel="current channel")) async def test_silence_voice(self): """Tests that the correct message was sent when a voice channel is muted without alerting.""" @@ -820,10 +819,10 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): with unittest.mock.patch.object(silence, "VOICE_CHANNELS") as mock_voice_channels: mock_voice_channels.get.return_value = self.text_channels[1].id - message = "This should show up as current channel." + message = "This should show up as {channel}." await self.cog.send_message(message, self.text_channels[0], self.voice_channel, alert_target=True) - updated_message = message.replace("current channel", self.voice_channel.mention) + updated_message = message.format(channel=self.voice_channel.mention) self.text_channels[0].send.assert_awaited_once_with(updated_message) self.text_channels[1].send.assert_awaited_once_with(updated_message) @@ -834,10 +833,10 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): with unittest.mock.patch.object(silence, "VOICE_CHANNELS") as mock_voice_channels: mock_voice_channels.get.return_value = self.text_channels[1].id - message = "This should show up as current channel." + message = "This should show up as {channel}." await self.cog.send_message(message, self.text_channels[1], self.voice_channel, alert_target=True) - updated_message = message.replace("current channel", self.voice_channel.mention) + updated_message = message.format(channel=self.voice_channel.mention) self.text_channels[1].send.assert_awaited_once_with(updated_message) mock_voice_channels.get.assert_called_once_with(self.voice_channel.id) -- cgit v1.2.3 From 0fe7755733bafefe5f4dfba6458f4dbec27ac9f4 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Thu, 13 May 2021 00:32:58 +0300 Subject: Updates Silence To Accept Duration Or Channel Updates the silence command to accept the silence duration or channel as the first argument to the command. Updates tests. Signed-off-by: Hassan Abouelela --- bot/exts/moderation/silence.py | 35 +++++++++--- tests/bot/exts/moderation/test_silence.py | 91 ++++++++++++++++++++++++++++--- 2 files changed, 109 insertions(+), 17 deletions(-) diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index e5c96e76f..8e4ce7ae2 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -1,5 +1,6 @@ import json import logging +import typing from contextlib import suppress from datetime import datetime, timedelta, timezone from typing import Optional, OrderedDict, Union @@ -84,12 +85,8 @@ class SilenceNotifier(tasks.Loop): async def _select_lock_channel(args: OrderedDict[str, any]) -> TextOrVoiceChannel: """Passes the channel to be silenced to the resource lock.""" - channel = args["channel"] - if channel is not None: - return channel - - else: - return args["ctx"].channel + channel, _ = Silence.parse_silence_args(args["ctx"], args["duration_or_channel"], args["duration"]) + return channel class Silence(commands.Cog): @@ -155,8 +152,8 @@ class Silence(commands.Cog): async def silence( self, ctx: Context, + duration_or_channel: typing.Union[TextOrVoiceChannel, HushDurationConverter] = None, duration: HushDurationConverter = 10, - channel: TextOrVoiceChannel = None, *, kick: bool = False ) -> None: @@ -170,8 +167,8 @@ class Silence(commands.Cog): If `kick` is True, members will not be added back to the voice channel, and members will be unable to rejoin. """ await self._init_task - if channel is None: - channel = ctx.channel + channel, duration = self.parse_silence_args(ctx, duration_or_channel, duration) + channel_info = f"#{channel} ({channel.id})" log.debug(f"{ctx.author} is silencing channel {channel_info}.") @@ -198,6 +195,26 @@ class Silence(commands.Cog): formatted_message = MSG_SILENCE_SUCCESS.format(duration=duration) await self.send_message(formatted_message, ctx.channel, channel, alert_target=True) + @staticmethod + def parse_silence_args( + ctx: Context, + duration_or_channel: typing.Union[TextOrVoiceChannel, int], + duration: HushDurationConverter + ) -> typing.Tuple[TextOrVoiceChannel, int]: + """Helper method to parse the arguments of the silence command.""" + duration: int + + if duration_or_channel: + if isinstance(duration_or_channel, (TextChannel, VoiceChannel)): + channel = duration_or_channel + else: + channel = ctx.channel + duration = duration_or_channel + else: + channel = ctx.channel + + return channel, duration + async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, *, kick: bool = False) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" # Get the original channel overwrites diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index af6dd5a37..a7ea733c5 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -260,6 +260,81 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): self.assertEqual(member.move_to.call_count, 1 if member == failing_member else 2) +class SilenceArgumentParserTests(unittest.IsolatedAsyncioTestCase): + """Tests for the silence argument parser utility function.""" + + def setUp(self): + self.bot = MockBot() + self.cog = silence.Silence(self.bot) + self.cog._init_task = asyncio.Future() + self.cog._init_task.set_result(None) + + @autospec(silence.Silence, "send_message", pass_mocks=False) + @autospec(silence.Silence, "_set_silence_overwrites", return_value=False, pass_mocks=False) + @autospec(silence.Silence, "parse_silence_args") + async def test_command(self, parser_mock): + """Test that the command passes in the correct arguments for different calls.""" + test_cases = ( + (), + (15, ), + (MockTextChannel(),), + (MockTextChannel(), 15), + ) + + ctx = MockContext() + parser_mock.return_value = (ctx.channel, 10) + + for case in test_cases: + with self.subTest("Test command converters", args=case): + await self.cog.silence.callback(self.cog, ctx, *case) + + try: + first_arg = case[0] + except IndexError: + # Default value when the first argument is not passed + first_arg = None + + try: + second_arg = case[1] + except IndexError: + # Default value when the second argument is not passed + second_arg = 10 + + parser_mock.assert_called_with(ctx, first_arg, second_arg) + + async def test_no_arguments(self): + """Test the parser when no arguments are passed to the command.""" + ctx = MockContext() + channel, duration = self.cog.parse_silence_args(ctx, None, 10) + + self.assertEqual(ctx.channel, channel) + self.assertEqual(10, duration) + + async def test_channel_only(self): + """Test the parser when just the channel argument is passed.""" + expected_channel = MockTextChannel() + actual_channel, duration = self.cog.parse_silence_args(MockContext(), expected_channel, 10) + + self.assertEqual(expected_channel, actual_channel) + self.assertEqual(10, duration) + + async def test_duration_only(self): + """Test the parser when just the duration argument is passed.""" + ctx = MockContext() + channel, duration = self.cog.parse_silence_args(ctx, 15, 10) + + self.assertEqual(ctx.channel, channel) + self.assertEqual(15, duration) + + async def test_all_args(self): + """Test the parser when both channel and duration are passed.""" + expected_channel = MockTextChannel() + actual_channel, duration = self.cog.parse_silence_args(MockContext(), expected_channel, 15) + + self.assertEqual(expected_channel, actual_channel) + self.assertEqual(15, duration) + + @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) class RescheduleTests(unittest.IsolatedAsyncioTestCase): """Tests for the rescheduling of cached unsilences.""" @@ -387,7 +462,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): with self.subTest(was_silenced=was_silenced, target=target, message=message): with mock.patch.object(self.cog, "send_message") as send_message: ctx = MockContext() - await self.cog.silence.callback(self.cog, ctx, duration, target) + await self.cog.silence.callback(self.cog, ctx, target, duration) send_message.assert_called_once_with( message, ctx.channel, @@ -399,7 +474,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_sync_called(self, ctx, sync, kick): """Tests if silence command calls sync on a voice channel.""" channel = MockVoiceChannel() - await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=False) + await self.cog.silence.callback(self.cog, ctx, channel, 10, kick=False) sync.assert_awaited_once_with(self.cog, channel) kick.assert_not_called() @@ -408,7 +483,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_kick_called(self, ctx, sync, kick): """Tests if silence command calls kick on a voice channel.""" channel = MockVoiceChannel() - await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=True) + await self.cog.silence.callback(self.cog, ctx, channel, 10, kick=True) kick.assert_awaited_once_with(channel) sync.assert_not_called() @@ -417,7 +492,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_sync_not_called(self, ctx, sync, kick): """Tests that silence command does not call sync on a text channel.""" channel = MockTextChannel() - await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=False) + await self.cog.silence.callback(self.cog, ctx, channel, 10, kick=False) sync.assert_not_called() kick.assert_not_called() @@ -426,7 +501,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_kick_not_called(self, ctx, sync, kick): """Tests that silence command does not call kick on a text channel.""" channel = MockTextChannel() - await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=True) + await self.cog.silence.callback(self.cog, ctx, channel, 10, kick=True) sync.assert_not_called() kick.assert_not_called() @@ -515,7 +590,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_indefinite_added_to_notifier(self): """Channel was added to notifier if a duration was not set for the silence.""" with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): - await self.cog.silence.callback(self.cog, MockContext(), None) + await self.cog.silence.callback(self.cog, MockContext(), None, None) self.cog.notifier.add_channel.assert_called_once() async def test_silenced_not_added_to_notifier(self): @@ -547,7 +622,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_cached_indefinite_time(self): """A value of -1 was cached for a permanent silence.""" ctx = MockContext(channel=self.text_channel) - await self.cog.silence.callback(self.cog, ctx, None) + await self.cog.silence.callback(self.cog, ctx, None, None) self.cog.unsilence_timestamps.set.assert_awaited_once_with(ctx.channel.id, -1) async def test_scheduled_task(self): @@ -563,7 +638,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_permanent_not_scheduled(self): """A task was not scheduled for a permanent silence.""" ctx = MockContext(channel=self.text_channel) - await self.cog.silence.callback(self.cog, ctx, None) + await self.cog.silence.callback(self.cog, ctx, None, None) self.cog.scheduler.schedule_later.assert_not_called() -- cgit v1.2.3 From 50294816787562abd5f3f984f513d039612d2f3b Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Thu, 13 May 2021 06:13:31 +0300 Subject: Updates Shh Command To Mirror Silence Updates the shh and unshh commands from the error handler to accept channel and kick arguments, to give them the same interface as the silence and unsilence command. Signed-off-by: Hassan Abouelela --- bot/exts/backend/error_handler.py | 27 ++++++++- tests/bot/exts/backend/test_error_handler.py | 84 ++++++++++++++++++++++++---- 2 files changed, 98 insertions(+), 13 deletions(-) diff --git a/bot/exts/backend/error_handler.py b/bot/exts/backend/error_handler.py index d8de177f5..a3c04437f 100644 --- a/bot/exts/backend/error_handler.py +++ b/bot/exts/backend/error_handler.py @@ -3,7 +3,7 @@ import logging import typing as t from discord import Embed -from discord.ext.commands import Cog, Context, errors +from discord.ext.commands import ChannelNotFound, Cog, Context, TextChannelConverter, VoiceChannelConverter, errors from sentry_sdk import push_scope from bot.api import ResponseCodeError @@ -115,8 +115,10 @@ class ErrorHandler(Cog): Return bool depending on success of command. """ command = ctx.invoked_with.lower() + args = ctx.message.content.lower().split(" ") silence_command = self.bot.get_command("silence") 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.") @@ -124,11 +126,30 @@ class ErrorHandler(Cog): except errors.CommandError: log.debug("Cancelling attempt to invoke silence/unsilence due to failed checks.") return False + + # Parse optional args + channel = None + duration = min(command.count("h") * 2, 15) + kick = False + + if len(args) > 1: + # Parse channel + for converter in (TextChannelConverter(), VoiceChannelConverter()): + try: + channel = await converter.convert(ctx, args[1]) + break + except ChannelNotFound: + continue + + if len(args) > 2 and channel is not None: + # Parse kick + kick = args[2].lower() == "true" + if command.startswith("shh"): - await ctx.invoke(silence_command, duration=min(command.count("h")*2, 15)) + await ctx.invoke(silence_command, duration_or_channel=channel, duration=duration, kick=kick) return True elif command.startswith("unshh"): - await ctx.invoke(self.bot.get_command("unsilence")) + await ctx.invoke(self.bot.get_command("unsilence"), channel=channel) return True return False diff --git a/tests/bot/exts/backend/test_error_handler.py b/tests/bot/exts/backend/test_error_handler.py index bd4fb5942..37e8108fc 100644 --- a/tests/bot/exts/backend/test_error_handler.py +++ b/tests/bot/exts/backend/test_error_handler.py @@ -9,7 +9,7 @@ from bot.exts.backend.error_handler import ErrorHandler, setup from bot.exts.info.tags import Tags from bot.exts.moderation.silence import Silence from bot.utils.checks import InWhitelistCheckFailure -from tests.helpers import MockBot, MockContext, MockGuild, MockRole +from tests.helpers import MockBot, MockContext, MockGuild, MockRole, MockTextChannel class ErrorHandlerTests(unittest.IsolatedAsyncioTestCase): @@ -226,8 +226,8 @@ class TrySilenceTests(unittest.IsolatedAsyncioTestCase): self.bot.get_command.return_value.can_run = AsyncMock(side_effect=errors.CommandError()) self.assertFalse(await self.cog.try_silence(self.ctx)) - async def test_try_silence_silencing(self): - """Should run silence command with correct arguments.""" + async def test_try_silence_silence_duration(self): + """Should run silence command with correct duration argument.""" self.bot.get_command.return_value.can_run = AsyncMock(return_value=True) test_cases = ("shh", "shhh", "shhhhhh", "shhhhhhhhhhhhhhhhhhh") @@ -238,21 +238,85 @@ class TrySilenceTests(unittest.IsolatedAsyncioTestCase): self.assertTrue(await self.cog.try_silence(self.ctx)) self.ctx.invoke.assert_awaited_once_with( self.bot.get_command.return_value, - duration=min(case.count("h")*2, 15) + duration_or_channel=None, + duration=min(case.count("h")*2, 15), + kick=False ) + async def test_try_silence_silence_arguments(self): + """Should run silence with the correct channel, duration, and kick arguments.""" + self.bot.get_command.return_value.can_run = AsyncMock(return_value=True) + + test_cases = ( + (MockTextChannel(), None), # None represents the case when no argument is passed + (MockTextChannel(), False), + (MockTextChannel(), True) + ) + + for channel, kick in test_cases: + with self.subTest(kick=kick, channel=channel): + self.ctx.reset_mock() + self.ctx.invoked_with = "shh" + + self.ctx.message.content = f"!shh {channel.name} {kick if kick is not None else ''}" + self.ctx.guild.text_channels = [channel] + + self.assertTrue(await self.cog.try_silence(self.ctx)) + self.ctx.invoke.assert_awaited_once_with( + self.bot.get_command.return_value, + duration_or_channel=channel, + duration=4, + kick=(kick if kick is not None else False) + ) + + async def test_try_silence_silence_message(self): + """If the words after the command could not be converted to a channel, None should be passed as channel.""" + self.bot.get_command.return_value.can_run = AsyncMock(return_value=True) + self.ctx.invoked_with = "shh" + self.ctx.message.content = "!shh not_a_channel true" + + self.assertTrue(await self.cog.try_silence(self.ctx)) + self.ctx.invoke.assert_awaited_once_with( + self.bot.get_command.return_value, + duration_or_channel=None, + duration=4, + kick=False + ) + async def test_try_silence_unsilence(self): - """Should call unsilence command.""" + """Should call unsilence command with correct duration and channel arguments.""" self.silence.silence.can_run = AsyncMock(return_value=True) - test_cases = ("unshh", "unshhhhh", "unshhhhhhhhh") + test_cases = ( + ("unshh", None), + ("unshhhhh", None), + ("unshhhhhhhhh", None), + ("unshh", MockTextChannel()) + ) - for case in test_cases: - with self.subTest(message=case): + for invoke, channel in test_cases: + with self.subTest(message=invoke, channel=channel): self.bot.get_command.side_effect = (self.silence.silence, self.silence.unsilence) self.ctx.reset_mock() - self.ctx.invoked_with = case + + self.ctx.invoked_with = invoke + self.ctx.message.content = f"!{invoke}" + if channel is not None: + self.ctx.message.content += f" {channel.name}" + self.ctx.guild.text_channels = [channel] + self.assertTrue(await self.cog.try_silence(self.ctx)) - self.ctx.invoke.assert_awaited_once_with(self.silence.unsilence) + self.ctx.invoke.assert_awaited_once_with(self.silence.unsilence, channel=channel) + + async def test_try_silence_unsilence_message(self): + """If the words after the command could not be converted to a channel, None should be passed as channel.""" + self.silence.silence.can_run = AsyncMock(return_value=True) + self.bot.get_command.side_effect = (self.silence.silence, self.silence.unsilence) + + self.ctx.invoked_with = "unshh" + self.ctx.message.content = "!unshh not_a_channel" + + self.assertTrue(await self.cog.try_silence(self.ctx)) + self.ctx.invoke.assert_awaited_once_with(self.silence.unsilence, channel=None) async def test_try_silence_no_match(self): """Should return `False` when message don't match.""" -- cgit v1.2.3 From f92338ef18d1bc5d11405a5d9e6ede4f9e080110 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 5 Jun 2021 14:04:06 +0300 Subject: Properly Handles Indefinite Silences Fixes a bug that stopped the duration `forever` from getting used as a valid duration for silence. Signed-off-by: Hassan Abouelela --- bot/converters.py | 6 +++--- bot/exts/moderation/silence.py | 7 +++++-- tests/bot/exts/moderation/test_silence.py | 7 +++++++ tests/bot/test_converters.py | 2 +- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index 2a3943831..595809517 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -416,11 +416,11 @@ class HushDurationConverter(Converter): MINUTES_RE = re.compile(r"(\d+)(?:M|m|$)") - async def convert(self, ctx: Context, argument: str) -> t.Optional[int]: + async def convert(self, ctx: Context, argument: str) -> 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. + If `"forever"` is passed, -1 is returned; otherwise an int of the extracted time. Accepted formats are: * , * m, @@ -428,7 +428,7 @@ class HushDurationConverter(Converter): * forever. """ if argument == "forever": - return None + return -1 match = self.MINUTES_RE.match(argument) if not match: raise BadArgument(f"{argument} is not a valid minutes duration.") diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 8e4ce7ae2..8025f3df6 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -200,9 +200,9 @@ class Silence(commands.Cog): ctx: Context, duration_or_channel: typing.Union[TextOrVoiceChannel, int], duration: HushDurationConverter - ) -> typing.Tuple[TextOrVoiceChannel, int]: + ) -> typing.Tuple[TextOrVoiceChannel, Optional[int]]: """Helper method to parse the arguments of the silence command.""" - duration: int + duration: Optional[int] if duration_or_channel: if isinstance(duration_or_channel, (TextChannel, VoiceChannel)): @@ -213,6 +213,9 @@ class Silence(commands.Cog): else: channel = ctx.channel + if duration == -1: + duration = None + return channel, duration async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, *, kick: bool = False) -> bool: diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index a7ea733c5..59a5893ef 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -641,6 +641,13 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.silence.callback(self.cog, ctx, None, None) self.cog.scheduler.schedule_later.assert_not_called() + async def test_indefinite_silence(self): + """Test silencing a channel forever.""" + with mock.patch.object(self.cog, "_schedule_unsilence") as unsilence: + ctx = MockContext(channel=self.text_channel) + await self.cog.silence.callback(self.cog, ctx, -1) + unsilence.assert_awaited_once_with(ctx, ctx.channel, None) + @autospec(silence.Silence, "unsilence_timestamps", pass_mocks=False) class UnsilenceTests(unittest.IsolatedAsyncioTestCase): diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index 4af84dde5..2a1c4e543 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -291,7 +291,7 @@ class ConverterTests(unittest.IsolatedAsyncioTestCase): ("10", 10), ("5m", 5), ("5M", 5), - ("forever", None), + ("forever", -1), ) converter = HushDurationConverter() for minutes_string, expected_minutes in test_values: -- cgit v1.2.3