From df897e57ea0dbbaeb3e2a07a1e443c5b2df34c36 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 30 Nov 2020 22:26:23 +0300 Subject: Adds Member Checks Before Changing Voice Adds a check that checks if the user object is an instance of guild member, before performing guild operations. Adds tests. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/infraction/test_infractions.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/infraction/test_infractions.py b/tests/bot/exts/moderation/infraction/test_infractions.py index bf557a484..4ba5a4feb 100644 --- a/tests/bot/exts/moderation/infraction/test_infractions.py +++ b/tests/bot/exts/moderation/infraction/test_infractions.py @@ -4,7 +4,7 @@ from unittest.mock import AsyncMock, MagicMock, Mock, patch from bot.constants import Event from bot.exts.moderation.infraction.infractions import Infractions -from tests.helpers import MockBot, MockContext, MockGuild, MockMember, MockRole +from tests.helpers import MockBot, MockContext, MockGuild, MockMember, MockRole, MockUser class TruncationTests(unittest.IsolatedAsyncioTestCase): @@ -164,6 +164,19 @@ class VoiceBanTests(unittest.IsolatedAsyncioTestCase): ) self.cog.apply_infraction.assert_awaited_once_with(self.ctx, {"foo": "bar"}, self.user, "my_return_value") + @patch("bot.exts.moderation.infraction._utils.get_active_infraction", return_value=None) + @patch("bot.exts.moderation.infraction._utils.post_infraction") + @patch("bot.exts.moderation.infraction.infractions.Infractions.apply_infraction") + async def test_voice_ban_user_left_guild(self, apply_infraction_mock, post_infraction_mock, _): + """Should voice ban user that left the guild without throwing an error.""" + infraction = {"foo": "bar"} + post_infraction_mock.return_value = {"foo": "bar"} + + user = MockUser() + await self.cog.voiceban(self.cog, self.ctx, user, reason=None) + post_infraction_mock.assert_called_once_with(self.ctx, user, "voice_ban", None, active=True) + apply_infraction_mock.assert_called_once_with(self.ctx, infraction, user, None) + async def test_voice_unban_user_not_found(self): """Should include info to return dict when user was not found from guild.""" self.guild.get_member.return_value = None -- cgit v1.2.3 From 37bfd7569cdeb0900ee131c22f67aed3f78692ba Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Wed, 20 Jan 2021 20:19:47 +0300 Subject: Cleans Up Tests --- tests/bot/exts/moderation/infraction/test_infractions.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/infraction/test_infractions.py b/tests/bot/exts/moderation/infraction/test_infractions.py index 4ba5a4feb..13efee054 100644 --- a/tests/bot/exts/moderation/infraction/test_infractions.py +++ b/tests/bot/exts/moderation/infraction/test_infractions.py @@ -3,8 +3,9 @@ import unittest from unittest.mock import AsyncMock, MagicMock, Mock, patch from bot.constants import Event +from bot.exts.moderation.infraction import _utils from bot.exts.moderation.infraction.infractions import Infractions -from tests.helpers import MockBot, MockContext, MockGuild, MockMember, MockRole, MockUser +from tests.helpers import MockBot, MockContext, MockGuild, MockMember, MockRole, MockUser, autospec class TruncationTests(unittest.IsolatedAsyncioTestCase): @@ -164,9 +165,8 @@ class VoiceBanTests(unittest.IsolatedAsyncioTestCase): ) self.cog.apply_infraction.assert_awaited_once_with(self.ctx, {"foo": "bar"}, self.user, "my_return_value") - @patch("bot.exts.moderation.infraction._utils.get_active_infraction", return_value=None) - @patch("bot.exts.moderation.infraction._utils.post_infraction") - @patch("bot.exts.moderation.infraction.infractions.Infractions.apply_infraction") + @autospec(_utils, "post_infraction", "get_active_infraction", return_value=None) + @autospec(Infractions, "apply_infraction") async def test_voice_ban_user_left_guild(self, apply_infraction_mock, post_infraction_mock, _): """Should voice ban user that left the guild without throwing an error.""" infraction = {"foo": "bar"} @@ -175,7 +175,7 @@ class VoiceBanTests(unittest.IsolatedAsyncioTestCase): user = MockUser() await self.cog.voiceban(self.cog, self.ctx, user, reason=None) post_infraction_mock.assert_called_once_with(self.ctx, user, "voice_ban", None, active=True) - apply_infraction_mock.assert_called_once_with(self.ctx, infraction, user, None) + apply_infraction_mock.assert_called_once_with(self.cog, self.ctx, infraction, user, None) async def test_voice_unban_user_not_found(self): """Should include info to return dict when user was not found from guild.""" -- cgit v1.2.3 From 06c5eba63f735237b44f2b1da9944bc7cddd2177 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Thu, 21 Jan 2021 01:26:41 +0300 Subject: Restructures Voice Ban Action Updates the voice ban action so the infraction pardoning is always run, and so all operations are handled in the _scheduler. Updates tests. --- bot/exts/moderation/infraction/infractions.py | 11 ++++--- .../exts/moderation/infraction/test_infractions.py | 37 +++++++++++++++------- 2 files changed, 32 insertions(+), 16 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index 78c326c47..be4327bb0 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -355,14 +355,15 @@ class Infractions(InfractionScheduler, commands.Cog): if reason: reason = textwrap.shorten(reason, width=512, placeholder="...") - action = None + async def action() -> None: + # Skip members that left the server + if not isinstance(user, Member): + return - # Skip members that left the server - if isinstance(user, Member): await user.move_to(None, reason="Disconnected from voice to apply voiceban.") - action = user.remove_roles(self._voice_verified_role, reason=reason) + await user.remove_roles(self._voice_verified_role, reason=reason) - await self.apply_infraction(ctx, infraction, user, action) + await self.apply_infraction(ctx, infraction, user, action()) # endregion # region: Base pardon functions diff --git a/tests/bot/exts/moderation/infraction/test_infractions.py b/tests/bot/exts/moderation/infraction/test_infractions.py index 13efee054..86c2617ea 100644 --- a/tests/bot/exts/moderation/infraction/test_infractions.py +++ b/tests/bot/exts/moderation/infraction/test_infractions.py @@ -1,6 +1,7 @@ +import inspect import textwrap import unittest -from unittest.mock import AsyncMock, MagicMock, Mock, patch +from unittest.mock import ANY, AsyncMock, MagicMock, Mock, patch from bot.constants import Event from bot.exts.moderation.infraction import _utils @@ -133,20 +134,29 @@ class VoiceBanTests(unittest.IsolatedAsyncioTestCase): self.assertIsNone(await self.cog.apply_voice_ban(self.ctx, self.user, "foobar")) self.cog.mod_log.ignore.assert_called_once_with(Event.member_update, self.user.id) + async def action_tester(self, action, reason: str) -> None: + """Helper method to test voice ban action.""" + self.assertTrue(inspect.iscoroutine(action)) + await action + + self.user.move_to.assert_called_once_with(None, reason=ANY) + self.user.remove_roles.assert_called_once_with(self.cog._voice_verified_role, reason=reason) + @patch("bot.exts.moderation.infraction.infractions._utils.post_infraction") @patch("bot.exts.moderation.infraction.infractions._utils.get_active_infraction") async def test_voice_ban_apply_infraction(self, get_active_infraction, post_infraction_mock): """Should ignore Voice Verified role removing.""" self.cog.mod_log.ignore = MagicMock() self.cog.apply_infraction = AsyncMock() - self.user.remove_roles = MagicMock(return_value="my_return_value") get_active_infraction.return_value = None post_infraction_mock.return_value = {"foo": "bar"} - self.assertIsNone(await self.cog.apply_voice_ban(self.ctx, self.user, "foobar")) - self.user.remove_roles.assert_called_once_with(self.cog._voice_verified_role, reason="foobar") - self.cog.apply_infraction.assert_awaited_once_with(self.ctx, {"foo": "bar"}, self.user, "my_return_value") + reason = "foobar" + self.assertIsNone(await self.cog.apply_voice_ban(self.ctx, self.user, reason)) + self.cog.apply_infraction.assert_awaited_once_with(self.ctx, {"foo": "bar"}, self.user, ANY) + + await self.action_tester(self.cog.apply_infraction.call_args[0][-1], reason) @patch("bot.exts.moderation.infraction.infractions._utils.post_infraction") @patch("bot.exts.moderation.infraction.infractions._utils.get_active_infraction") @@ -154,16 +164,16 @@ class VoiceBanTests(unittest.IsolatedAsyncioTestCase): """Should truncate reason for voice ban.""" self.cog.mod_log.ignore = MagicMock() self.cog.apply_infraction = AsyncMock() - self.user.remove_roles = MagicMock(return_value="my_return_value") get_active_infraction.return_value = None post_infraction_mock.return_value = {"foo": "bar"} self.assertIsNone(await self.cog.apply_voice_ban(self.ctx, self.user, "foobar" * 3000)) - self.user.remove_roles.assert_called_once_with( - self.cog._voice_verified_role, reason=textwrap.shorten("foobar" * 3000, 512, placeholder="...") - ) - self.cog.apply_infraction.assert_awaited_once_with(self.ctx, {"foo": "bar"}, self.user, "my_return_value") + self.cog.apply_infraction.assert_awaited_once_with(self.ctx, {"foo": "bar"}, self.user, ANY) + + # Test action + action = self.cog.apply_infraction.call_args[0][-1] + await self.action_tester(action, textwrap.shorten("foobar" * 3000, 512, placeholder="...")) @autospec(_utils, "post_infraction", "get_active_infraction", return_value=None) @autospec(Infractions, "apply_infraction") @@ -175,7 +185,12 @@ class VoiceBanTests(unittest.IsolatedAsyncioTestCase): user = MockUser() await self.cog.voiceban(self.cog, self.ctx, user, reason=None) post_infraction_mock.assert_called_once_with(self.ctx, user, "voice_ban", None, active=True) - apply_infraction_mock.assert_called_once_with(self.cog, self.ctx, infraction, user, None) + apply_infraction_mock.assert_called_once_with(self.cog, self.ctx, infraction, user, ANY) + + # Test action + action = self.cog.apply_infraction.call_args[0][-1] + self.assertTrue(inspect.iscoroutine(action)) + await action async def test_voice_unban_user_not_found(self): """Should include info to return dict when user was not found from guild.""" -- cgit v1.2.3