diff --git a/bot/exts/filtering/_settings_types/actions/infraction_and_notification.py b/bot/exts/filtering/_settings_types/actions/infraction_and_notification.py index 359aa7bc34..163df16ab2 100644 --- a/bot/exts/filtering/_settings_types/actions/infraction_and_notification.py +++ b/bot/exts/filtering/_settings_types/actions/infraction_and_notification.py @@ -15,6 +15,7 @@ from bot.exts.filtering._filter_context import FilterContext from bot.exts.filtering._settings_types.settings_entry import ActionEntry from bot.exts.filtering._utils import CustomIOField, FakeContext +from bot.exts.moderation.infraction import _utils as infraction_utils from bot.utils.time import humanize_delta, parse_duration_string, relativedelta_to_timedelta log = get_logger(__name__) @@ -87,14 +88,14 @@ async def invoke( alerts_channel: discord.TextChannel, duration: InfractionDuration, reason: str - ) -> None: + ) -> str | None: """Invokes the command matching the infraction name.""" command_name = self.name.lower() command = bot_module.instance.get_command(command_name) if not command: await alerts_channel.send(f":warning: Could not apply {command_name} to {user.mention}: command not found.") log.warning(f":warning: Could not apply {command_name} to {user.mention}: command not found.") - return + return None if isinstance(user, discord.User): # For example because a message was sent in a DM. member = await get_or_fetch_member(channel.guild, user.id) @@ -105,14 +106,20 @@ async def invoke( f"The user {user} were set to receive an automatic {command_name}, " "but they were not found in the guild." ) - return + return None ctx = FakeContext(message, channel, command) + if self is Infraction.BAN: + active_infraction = await infraction_utils.get_active_infraction(ctx, user, "ban", send_msg=False) + if active_infraction: + return "already banned" + if self.name in ("KICK", "WARNING", "WATCH", "NOTE"): await command(ctx, user, reason=reason or None) else: duration = arrow.utcnow().datetime + duration.value if duration.value else None await command(ctx, user, duration, reason=reason or None) + return passive_form[self.name] class InfractionAndNotification(ActionEntry): @@ -203,10 +210,11 @@ async def action(self, ctx: FilterContext) -> None: log.error(f"Unable to apply infraction as the context channel {channel} can't be found.") return - await self.infraction_type.invoke( + infraction_action = await self.infraction_type.invoke( ctx.author, ctx.message, channel, alerts_channel, self.infraction_duration, self.infraction_reason ) - ctx.action_descriptions.append(passive_form[self.infraction_type.name]) + if infraction_action: + ctx.action_descriptions.append(infraction_action) def union(self, other: Self) -> Self: """ diff --git a/bot/exts/filtering/_ui/ui.py b/bot/exts/filtering/_ui/ui.py index 5382f846b2..bc8d24b976 100644 --- a/bot/exts/filtering/_ui/ui.py +++ b/bot/exts/filtering/_ui/ui.py @@ -616,7 +616,7 @@ class AlertView(discord.ui.View): def __init__(self, ctx: FilterContext, triggered_filters: dict[FilterList, list[str]] | None = None): super().__init__(timeout=ALERT_VIEW_TIMEOUT) self.ctx = ctx - if "banned" in self.ctx.action_descriptions: + if {"banned", "already banned"} & set(self.ctx.action_descriptions): # If the user has already been banned, do not attempt to add phishing button since the URL or guild invite # is probably already added as a filter return diff --git a/tests/bot/exts/filtering/test_settings_entries.py b/tests/bot/exts/filtering/test_settings_entries.py index ef00aaf424..8e0a3791e3 100644 --- a/tests/bot/exts/filtering/test_settings_entries.py +++ b/tests/bot/exts/filtering/test_settings_entries.py @@ -1,5 +1,5 @@ import unittest -from unittest.mock import patch +from unittest.mock import AsyncMock, patch from bot.constants import Roles from bot.exts.filtering._filter_context import Event, FilterContext @@ -238,3 +238,66 @@ def test_clean_ban_mentions_removes_moderator_and_broad_mentions(self, resolve_m cleaned = _clean_ban_mentions(mentions) self.assertSetEqual(cleaned, {"other-role", "12345"}) + + +class InfractionActionTests(unittest.IsolatedAsyncioTestCase): + """Tests for infraction action behavior in filtering.""" + + @patch("bot.exts.filtering._settings_types.actions.infraction_and_notification.infraction_utils.get_active_infraction") + @patch("bot.exts.filtering._settings_types.actions.infraction_and_notification.bot_module.instance") + async def test_ban_action_marks_already_banned_when_active_ban_exists(self, bot_instance, get_active_infraction): + """A pre-existing active ban should be reported as already banned without invoking the ban command.""" + member = MockMember(id=123) + channel = MockTextChannel(id=345) + alerts_channel = MockTextChannel(id=999) + message = MockMessage(author=member, channel=channel) + ctx = FilterContext(Event.MESSAGE, member, channel, "", message) + + ban_command = AsyncMock() + bot_instance.get_command.return_value = ban_command + bot_instance.get_channel.return_value = alerts_channel + get_active_infraction.return_value = {"id": 42, "type": "ban"} + + action = InfractionAndNotification( + infraction_type="BAN", + infraction_reason="reason", + infraction_duration=InfractionDuration(0), + dm_content="", + dm_embed="", + infraction_channel=0, + ) + + await action.action(ctx) + + self.assertEqual(ctx.action_descriptions, ["already banned"]) + ban_command.assert_not_awaited() + get_active_infraction.assert_awaited_once() + + @patch("bot.exts.filtering._settings_types.actions.infraction_and_notification.infraction_utils.get_active_infraction") + @patch("bot.exts.filtering._settings_types.actions.infraction_and_notification.bot_module.instance") + async def test_ban_action_marks_banned_when_no_active_ban(self, bot_instance, get_active_infraction): + """A successful ban path should preserve the existing banned action description.""" + member = MockMember(id=123) + channel = MockTextChannel(id=345) + alerts_channel = MockTextChannel(id=999) + message = MockMessage(author=member, channel=channel) + ctx = FilterContext(Event.MESSAGE, member, channel, "", message) + + ban_command = AsyncMock() + bot_instance.get_command.return_value = ban_command + bot_instance.get_channel.return_value = alerts_channel + get_active_infraction.return_value = None + + action = InfractionAndNotification( + infraction_type="BAN", + infraction_reason="reason", + infraction_duration=InfractionDuration(0), + dm_content="", + dm_embed="", + infraction_channel=0, + ) + + await action.action(ctx) + + self.assertEqual(ctx.action_descriptions, ["banned"]) + ban_command.assert_awaited_once()