diff --git a/src/cmds/core/ban.py b/src/cmds/core/ban.py index 18d98c3..67699ef 100644 --- a/src/cmds/core/ban.py +++ b/src/cmds/core/ban.py @@ -31,13 +31,14 @@ async def ban( self, ctx: ApplicationContext, user: discord.Member, reason: str, evidence: str = None ) -> Interaction | WebhookMessage: """Ban a user from the server permanently.""" + await ctx.defer(ephemeral=False) member = await self.bot.get_member_or_user(ctx.guild, user.id) if not member: - return await ctx.respond(f"User {user} not found.") + return await ctx.followup.send(f"User {user} not found.") response = await ban_member( self.bot, ctx.guild, member, "500w", reason, evidence, ctx.user, needs_approval=False ) - return await ctx.respond(response.message, delete_after=response.delete_after) + return await ctx.followup.send(response.message, delete_after=response.delete_after) @slash_command( guild_ids=settings.guild_ids, description="Ban a user from the server temporarily." @@ -50,13 +51,14 @@ async def tempban( self, ctx: ApplicationContext, user: discord.Member, duration: str, reason: str, evidence: str = None ) -> Interaction | WebhookMessage: """Ban a user from the server temporarily.""" + await ctx.defer(ephemeral=False) member = await self.bot.get_member_or_user(ctx.guild, user.id) if not member: - return await ctx.respond(f"User {user} not found.") + return await ctx.followup.send(f"User {user} not found.") response = await ban_member( self.bot, ctx.guild, member, duration, reason, evidence, ctx.user, needs_approval=True ) - return await ctx.respond(response.message, delete_after=response.delete_after) + return await ctx.followup.send(response.message, delete_after=response.delete_after) @slash_command(guild_ids=settings.guild_ids, description="Unbans a user from the server.") @has_any_role( @@ -156,11 +158,12 @@ async def dispute(self, ctx: ApplicationContext, ban_id: int, duration: str) -> @has_any_role(*settings.role_groups.get("ALL_ADMINS"), *settings.role_groups.get("ALL_MODS")) async def warn(self, ctx: ApplicationContext, user: discord.Member, reason: str) -> Interaction | WebhookMessage: """Warns a user of an action. Adds no weight but DMs the user about the warning and the reason why.""" + await ctx.defer(ephemeral=False) member = await self.bot.get_member_or_user(ctx.guild, user.id) if not member: - return await ctx.respond(f"User {user} not found.") + return await ctx.followup.send(f"User {user} not found.") response = await add_infraction(ctx.guild, member, 0, reason, ctx.user) - return await ctx.respond(response.message, delete_after=response.delete_after) + return await ctx.followup.send(response.message, delete_after=response.delete_after) @slash_command( guild_ids=settings.guild_ids, @@ -171,11 +174,12 @@ async def strike( self, ctx: ApplicationContext, user: discord.Member, weight: int, reason: str ) -> Interaction | WebhookMessage: """Strike the user with the selected weight. DMs the user about the strike and the reason why.""" + await ctx.defer(ephemeral=False) member = await self.bot.get_member_or_user(ctx.guild, user.id) if not member: - return await ctx.respond(f"User {user} not found.") + return await ctx.followup.send(f"User {user} not found.") response = await add_infraction(ctx.guild, member, weight, reason, ctx.user) - return await ctx.respond(response.message, delete_after=response.delete_after) + return await ctx.followup.send(response.message, delete_after=response.delete_after) @slash_command( guild_ids=settings.guild_ids, diff --git a/src/cmds/core/user.py b/src/cmds/core/user.py index bd5218c..1e2cc10 100644 --- a/src/cmds/core/user.py +++ b/src/cmds/core/user.py @@ -75,17 +75,18 @@ async def bad_name(self, ctx: ApplicationContext, user: Member) -> Interaction | async def kick(self, ctx: ApplicationContext, user: Member, reason: str, evidence: str = None) \ -> Interaction | WebhookMessage: """Kick a user from the server.""" + await ctx.defer(ephemeral=False) member = await self.bot.get_member_or_user(ctx.guild, user.id) - if not isinstance(member, discord.Member): - return await ctx.respond("User seems to have already left the server.") if not member: - return await ctx.respond(f"User {user} not found.") + return await ctx.followup.send(f"User {user} not found.") + if not isinstance(member, discord.Member): + return await ctx.followup.send("User seems to have already left the server.") if member_is_staff(member): - return await ctx.respond("You cannot kick another staff member.") + return await ctx.followup.send("You cannot kick another staff member.") if member.bot: - return await ctx.respond("You cannot kick a bot.") + return await ctx.followup.send("You cannot kick a bot.") if ctx.user.id == member.id: - return await ctx.respond("You cannot kick yourself.") + return await ctx.followup.send("You cannot kick yourself.") if len(reason) == 0: reason = "No reason given..." @@ -93,20 +94,20 @@ async def kick(self, ctx: ApplicationContext, user: Member, reason: str, evidenc try: await member.send(f"You have been kicked from {ctx.guild.name} for the following reason:\n>>> {reason}\n") except Forbidden as ex: - await ctx.respond( + await ctx.followup.send( "Could not DM member due to privacy settings, however will still attempt to kick them..." ) logger.warning(f"HTTPException when trying to unban user with ID {user.id}: {ex}") except HTTPException as ex: logger.warning(f"HTTPException when trying to unban user with ID {user.id}: {ex}") - return await ctx.respond( + return await ctx.followup.send( "Here's a 400 Bad Request for you. Just like when you tried to ask me out, last week.", ) await ctx.guild.kick(user=member, reason=reason) infraction_reason = f"Previously kicked for: {reason} - Evidence: {evidence}" await add_infraction(ctx.guild, member, 0, infraction_reason, ctx.user) - return await ctx.respond(f"{member.name} got the boot!") + return await ctx.followup.send(f"{member.name} got the boot!") def _match_role(self, role_name: str) -> Tuple[Union[int, None], Union[str, None]]: joinable = self.bot.role_manager.get_joinable_roles() if self.bot.role_manager else {} diff --git a/tests/helpers.py b/tests/helpers.py index 62cb583..9a7f775 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -496,7 +496,7 @@ class MockContext(CustomMockMixin, mock.MagicMock): """ spec_set = context_instance - additional_spec_asyncs = ("respond",) + additional_spec_asyncs = ("respond", "defer") def __init__(self, **kwargs) -> None: super().__init__(**kwargs) @@ -507,6 +507,8 @@ def __init__(self, **kwargs) -> None: self.channel = kwargs.get('channel', MockTextChannel()) self.message = kwargs.get('message', MockMessage()) self.respond = mock.AsyncMock() + self.followup = mock.MagicMock() + self.followup.send = mock.AsyncMock() dummy_attachment = { diff --git a/tests/src/cmds/core/test_ban.py b/tests/src/cmds/core/test_ban.py index e7a2bb9..6acc227 100644 --- a/tests/src/cmds/core/test_ban.py +++ b/tests/src/cmds/core/test_ban.py @@ -45,10 +45,23 @@ async def test_ban_success(self, ctx, bot): ban_member_mock.assert_called_once_with( bot, ctx.guild, user, "500w", "Any valid reason", "Some evidence", ctx.user, needs_approval=False ) - ctx.respond.assert_called_once_with( + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with( f"Member {user.display_name} has been banned permanently.", delete_after=0 ) + @pytest.mark.asyncio + async def test_ban_user_not_found(self, ctx, bot): + ctx.user = helpers.MockMember(id=1, name="Test User") + user = helpers.MockMember(id=2, name="Banned User") + bot.get_member_or_user.return_value = None + + cog = ban.BanCog(bot) + await cog.ban.callback(cog, ctx, user, "Any valid reason", "Some evidence") + + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with(f"User {user} not found.") + @pytest.mark.asyncio async def test_tempban_success(self, ctx, bot): ctx.user = helpers.MockMember(id=1, name="Test User") @@ -72,10 +85,23 @@ async def test_tempban_success(self, ctx, bot): ban_member_mock.assert_called_once_with( bot, ctx.guild, user, "5d", "Any valid reason", "Some evidence", ctx.user, needs_approval=True ) - ctx.respond.assert_called_once_with( + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with( f"Member {user.display_name} has been banned temporarily.", delete_after=0 ) + @pytest.mark.asyncio + async def test_tempban_user_not_found(self, ctx, bot): + ctx.user = helpers.MockMember(id=1, name="Test User") + user = helpers.MockMember(id=2, name="Banned User") + bot.get_member_or_user.return_value = None + + cog = ban.BanCog(bot) + await cog.tempban.callback(cog, ctx, user, "5d", "Any valid reason", "Some evidence") + + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with(f"User {user} not found.") + @pytest.mark.asyncio async def test_tempban_failed_with_wrong_duration(self, ctx, bot, guild): ctx.user = helpers.MockMember(id=1, name="Test User") @@ -102,7 +128,8 @@ async def test_tempban_failed_with_wrong_duration(self, ctx, bot, guild): ban_member_mock.assert_called_once_with( bot, ctx.guild, user, "5", "Any valid reason", "Some evidence", ctx.user, needs_approval=True ) - ctx.respond.assert_called_once_with( + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with( "Malformed duration. Please use duration units, (e.g. 12h, 14d, 5w).", delete_after=15 ) @@ -169,6 +196,11 @@ async def test_warn_success(self, ctx, bot): # Assertions add_infraction_mock.assert_called_once_with(ctx.guild, user, 0, "Any valid reason", ctx.user) + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with( + f"{user.mention} ({user.id}) has been warned with a strike weight of 0.", + delete_after=None, + ) @pytest.mark.asyncio async def test_warn_user_not_found(self, ctx, bot): @@ -180,7 +212,8 @@ async def test_warn_user_not_found(self, ctx, bot): await cog.warn.callback(cog, ctx, user, "Any valid reason") # Assertions - ctx.respond.assert_called_once_with(f"User {user} not found.") + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with(f"User {user} not found.") @pytest.mark.asyncio async def test_strike_success(self, ctx, bot): @@ -198,6 +231,11 @@ async def test_strike_success(self, ctx, bot): # Assertions add_infraction_mock.assert_called_once_with(ctx.guild, user, 10, "Any valid reason", ctx.user) + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with( + f"{user.mention} ({user.id}) has been warned with a strike weight of 10.", + delete_after=None, + ) @pytest.mark.asyncio async def test_strike_user_not_found(self, ctx, bot): @@ -209,7 +247,8 @@ async def test_strike_user_not_found(self, ctx, bot): await cog.strike.callback(cog, ctx, user, 10, "Any valid reason") # Assertions - ctx.respond.assert_called_once_with(f"User {user} not found.") + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with(f"User {user} not found.") @pytest.mark.asyncio async def test_remove_infraction_success(self, ctx, bot): diff --git a/tests/src/cmds/core/test_user.py b/tests/src/cmds/core/test_user.py index 350de82..92eab40 100644 --- a/tests/src/cmds/core/test_user.py +++ b/tests/src/cmds/core/test_user.py @@ -1,11 +1,20 @@ from unittest.mock import AsyncMock, patch import pytest +from discord.errors import Forbidden, HTTPException from src.cmds.core import user from tests import helpers +class MockResponse: + def __init__(self, status): + self.status = status + self.reason = "Forbidden" + self.code = status + self.text = "Cannot send messages to this user" + + class TestUserCog: """Test the `User` cog.""" @@ -35,7 +44,8 @@ async def test_kick_success(self, ctx, guild, bot, session): # Assertions ctx.guild.kick.assert_called_once_with(user=user_to_kick, reason="Violation of rules") - ctx.respond.assert_called_once_with("User to Kick got the boot!") + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with("User to Kick got the boot!") @pytest.mark.asyncio async def test_kick_fail_user_left(self, ctx, guild, bot, session): @@ -43,17 +53,119 @@ async def test_kick_fail_user_left(self, ctx, guild, bot, session): user_to_kick = helpers.MockMember(id=2, name="User to Kick", bot=False) ctx.guild = guild ctx.guild.kick = AsyncMock() - bot.get_member_or_user = AsyncMock(return_value=None) + # Still on Discord but no longer in the guild: User, not Member (None means user not found at all). + left_user = helpers.MockUser(id=user_to_kick.id, name="User to Kick") + bot.get_member_or_user = AsyncMock(return_value=left_user) - # Ensure the member_is_staff mock doesn't block execution with patch('src.cmds.core.user.member_is_staff', return_value=False): cog = user.UserCog(bot) await cog.kick.callback(cog, ctx, user_to_kick, "Violation of rules") - # Assertions bot.get_member_or_user.assert_called_once_with(ctx.guild, user_to_kick.id) - ctx.guild.kick.assert_not_called() # No kick should occur - ctx.respond.assert_called_once_with("User seems to have already left the server.") + ctx.guild.kick.assert_not_called() + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with("User seems to have already left the server.") + + @pytest.mark.asyncio + async def test_kick_fail_user_not_found(self, ctx, guild, bot, session): + ctx.user = helpers.MockMember(id=1, name="Test Moderator") + user_to_kick = helpers.MockMember(id=2, name="User to Kick", bot=False) + ctx.guild = guild + ctx.guild.kick = AsyncMock() + bot.get_member_or_user = AsyncMock(return_value=None) + + with patch('src.cmds.core.user.member_is_staff', return_value=False): + cog = user.UserCog(bot) + await cog.kick.callback(cog, ctx, user_to_kick, "Violation of rules") + + bot.get_member_or_user.assert_called_once_with(ctx.guild, user_to_kick.id) + ctx.guild.kick.assert_not_called() + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with(f"User {user_to_kick} not found.") + + @pytest.mark.asyncio + async def test_kick_fail_staff_member(self, ctx, guild, bot): + ctx.user = helpers.MockMember(id=1, name="Test Moderator") + member = helpers.MockMember(id=2, name="Staff Member", bot=False) + ctx.guild = guild + bot.get_member_or_user = AsyncMock(return_value=member) + + with patch('src.cmds.core.user.member_is_staff', return_value=True): + cog = user.UserCog(bot) + await cog.kick.callback(cog, ctx, member, "Violation of rules") + + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with("You cannot kick another staff member.") + + @pytest.mark.asyncio + async def test_kick_fail_bot_member(self, ctx, guild, bot): + ctx.user = helpers.MockMember(id=1, name="Test Moderator") + member = helpers.MockMember(id=2, name="Bot User", bot=True) + ctx.guild = guild + bot.get_member_or_user = AsyncMock(return_value=member) + + with patch('src.cmds.core.user.member_is_staff', return_value=False): + cog = user.UserCog(bot) + await cog.kick.callback(cog, ctx, member, "Violation of rules") + + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with("You cannot kick a bot.") + + @pytest.mark.asyncio + async def test_kick_fail_self_kick(self, ctx, guild, bot): + member = helpers.MockMember(id=1, name="Test Moderator", bot=False) + ctx.user = helpers.MockMember(id=1, name="Test Moderator") + ctx.guild = guild + bot.get_member_or_user = AsyncMock(return_value=member) + + with patch('src.cmds.core.user.member_is_staff', return_value=False): + cog = user.UserCog(bot) + await cog.kick.callback(cog, ctx, member, "Violation of rules") + + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.followup.send.assert_called_once_with("You cannot kick yourself.") + + @pytest.mark.asyncio + async def test_kick_http_exception_returns_error(self, ctx, guild, bot): + ctx.user = helpers.MockMember(id=1, name="Test Moderator") + member = helpers.MockMember(id=2, name="User to Kick", bot=False) + member.send = AsyncMock(side_effect=HTTPException(response=MockResponse(400), message="Bad request")) + ctx.guild = guild + ctx.guild.kick = AsyncMock() + bot.get_member_or_user = AsyncMock(return_value=member) + + with ( + patch('src.cmds.core.user.add_infraction', new_callable=AsyncMock), + patch('src.cmds.core.user.member_is_staff', return_value=False) + ): + cog = user.UserCog(bot) + await cog.kick.callback(cog, ctx, member, "Violation of rules") + + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.guild.kick.assert_not_called() + ctx.followup.send.assert_called_once_with( + "Here's a 400 Bad Request for you. Just like when you tried to ask me out, last week.", + ) + + @pytest.mark.asyncio + async def test_kick_forbidden_dm_sends_notice_and_continues(self, ctx, guild, bot): + ctx.user = helpers.MockMember(id=1, name="Test Moderator") + member = helpers.MockMember(id=2, name="User to Kick", bot=False) + member.send = AsyncMock(side_effect=Forbidden(response=MockResponse(403), message="Cannot DM")) + ctx.guild = guild + ctx.guild.kick = AsyncMock() + bot.get_member_or_user = AsyncMock(return_value=member) + + with ( + patch('src.cmds.core.user.add_infraction', new_callable=AsyncMock), + patch('src.cmds.core.user.member_is_staff', return_value=False) + ): + cog = user.UserCog(bot) + await cog.kick.callback(cog, ctx, member, "Violation of rules") + + ctx.defer.assert_awaited_once_with(ephemeral=False) + assert ctx.followup.send.await_count == 2 + ctx.guild.kick.assert_called_once_with(user=member, reason="Violation of rules") @pytest.mark.asyncio