From d541ebf7c8eaa1ef507f6cb0cd8133f0197b176d Mon Sep 17 00:00:00 2001 From: Tejas <98106526+ToxicBiohazard@users.noreply.github.com> Date: Mon, 13 Apr 2026 06:08:36 +0000 Subject: [PATCH 1/4] Refactor ban and kick commands to use followup responses instead of respond. Added defer calls for better user experience. Updated tests accordingly. --- src/cmds/core/ban.py | 20 ++++++++++++-------- src/cmds/core/user.py | 17 +++++++++-------- tests/helpers.py | 4 +++- tests/src/cmds/core/test_ban.py | 25 ++++++++++++++++++++----- tests/src/cmds/core/test_user.py | 6 ++++-- 5 files changed, 48 insertions(+), 24 deletions(-) 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..01e0b77 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.") + return await ctx.followup.send("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 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..772492e 100644 --- a/tests/src/cmds/core/test_ban.py +++ b/tests/src/cmds/core/test_ban.py @@ -45,7 +45,8 @@ 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 ) @@ -72,7 +73,8 @@ 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 ) @@ -102,7 +104,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 +172,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 +188,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 +207,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 +223,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..a3e84b4 100644 --- a/tests/src/cmds/core/test_user.py +++ b/tests/src/cmds/core/test_user.py @@ -35,7 +35,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): @@ -53,7 +54,8 @@ async def test_kick_fail_user_left(self, ctx, guild, bot, session): # 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.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 From 531ede589e4ab5487eba998954fdea4815d0915f Mon Sep 17 00:00:00 2001 From: Tejas <98106526+ToxicBiohazard@users.noreply.github.com> Date: Mon, 13 Apr 2026 06:25:37 +0000 Subject: [PATCH 2/4] fix: improve user kick command error handling and add tests for various failure scenarios - Removed redundant user not found check in the kick command. - Added tests for kicking staff members, bot members, self-kicking, and handling HTTP exceptions. - Enhanced test coverage for ban commands to include user not found scenarios. --- src/cmds/core/user.py | 2 - tests/src/cmds/core/test_ban.py | 24 +++++++++ tests/src/cmds/core/test_user.py | 93 ++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 2 deletions(-) diff --git a/src/cmds/core/user.py b/src/cmds/core/user.py index 01e0b77..2c5980b 100644 --- a/src/cmds/core/user.py +++ b/src/cmds/core/user.py @@ -79,8 +79,6 @@ async def kick(self, ctx: ApplicationContext, user: Member, reason: str, evidenc member = await self.bot.get_member_or_user(ctx.guild, user.id) if not isinstance(member, discord.Member): return await ctx.followup.send("User seems to have already left the server.") - if not member: - return await ctx.followup.send(f"User {user} not found.") if member_is_staff(member): return await ctx.followup.send("You cannot kick another staff member.") if member.bot: diff --git a/tests/src/cmds/core/test_ban.py b/tests/src/cmds/core/test_ban.py index 772492e..6acc227 100644 --- a/tests/src/cmds/core/test_ban.py +++ b/tests/src/cmds/core/test_ban.py @@ -50,6 +50,18 @@ async def test_ban_success(self, ctx, bot): 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") @@ -78,6 +90,18 @@ async def test_tempban_success(self, ctx, bot): 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") diff --git a/tests/src/cmds/core/test_user.py b/tests/src/cmds/core/test_user.py index a3e84b4..568f2aa 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.""" @@ -57,6 +66,90 @@ async def test_kick_fail_user_left(self, ctx, guild, bot, session): 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_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 async def test_user_stats(self, ctx, bot): From 70f1e8f837f9da7ad6c679647d14e47439a1d671 Mon Sep 17 00:00:00 2001 From: Tejas <98106526+ToxicBiohazard@users.noreply.github.com> Date: Wed, 15 Apr 2026 01:59:32 +0000 Subject: [PATCH 3/4] fix: add user not found handling in kick command - Implemented a check to handle cases where the user is not found in the server before proceeding with the kick operation. - This enhances error handling and improves user feedback during the kick command execution. --- src/cmds/core/user.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cmds/core/user.py b/src/cmds/core/user.py index 2c5980b..1e2cc10 100644 --- a/src/cmds/core/user.py +++ b/src/cmds/core/user.py @@ -77,6 +77,8 @@ async def kick(self, ctx: ApplicationContext, user: Member, reason: str, evidenc """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 member: + 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): From 8f9d5772da08d884dbc083e1fbb391c6d13da404 Mon Sep 17 00:00:00 2001 From: Tejas <98106526+ToxicBiohazard@users.noreply.github.com> Date: Wed, 15 Apr 2026 02:04:28 +0000 Subject: [PATCH 4/4] fix: enhance kick command tests for user not found scenarios - Added a new test to handle cases where the user to be kicked is not found in the guild. - Improved existing test to check for users who have left the server, ensuring accurate feedback is provided to moderators. - Updated assertions to confirm that no kick action is performed when the user is not found. --- tests/src/cmds/core/test_user.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/tests/src/cmds/core/test_user.py b/tests/src/cmds/core/test_user.py index 568f2aa..92eab40 100644 --- a/tests/src/cmds/core/test_user.py +++ b/tests/src/cmds/core/test_user.py @@ -53,19 +53,36 @@ 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.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")