diff --git a/bot.py b/bot.py index 5364b34..41ad744 100644 --- a/bot.py +++ b/bot.py @@ -10,7 +10,7 @@ # Add cogs from comandos.ping import Ping -from comandos.moderacion import Moderacion +from comandos.moderacion import Moderacion, ApproveButton, RejectButton from comandos.ayuda import Ayuda from comandos.flood import FloodSpam from comandos.limpia import Limpia @@ -40,8 +40,12 @@ intents.message_content = True bot = commands.Bot(command_prefix="%", intents=intents) -handler = logging.FileHandler(filename="bot.log", encoding="utf-8", mode="w") -discord.utils.setup_logging(level=logging.INFO, handler=handler) +file_handler = logging.FileHandler(filename="bot.log", encoding="utf-8", mode="w") +discord.utils.setup_logging(level=logging.INFO, handler=file_handler) +# Also mirror everything to the console - previously bot.log was the *only* +# handler, so running the bot attached to a terminal/screen session showed +# nothing there; every message only ever went to the file. +discord.utils.setup_logging(level=logging.INFO, handler=logging.StreamHandler()) logger = logging.getLogger(__name__) @@ -94,6 +98,12 @@ async def main(): row["message_id"]: row for row in data_mod if row["message_id"] not in ready_ids } + # Registers the Aprobar/Rechazar buttons as persistent - required for + # clicks on messages sent before this process started to keep working + # (see comandos/moderacion.py's ApproveButton docstring for why + # timeout=None alone doesn't already do this). + bot.add_dynamic_items(ApproveButton, RejectButton) + for cog_cls in COGS: await bot.add_cog(cog_cls(bot)) diff --git a/comandos/moderacion.py b/comandos/moderacion.py index ba0f32b..90e9ae1 100644 --- a/comandos/moderacion.py +++ b/comandos/moderacion.py @@ -50,6 +50,21 @@ class ValidatedPost: author: discord.User +def _pending_author(cog, message_id: int): + """Resolve a still-pending post's author live, at click time, from + ``bot.data_mod`` - used by the persistent Aprobar/Rechazar buttons below + so they never depend on the Python state of the moment the message was + first sent, which is gone after a bot restart.""" + mod_row = cog.bot.data_mod.get(str(message_id)) + if mod_row is None: + return None + return cog.bot.get_user(int(mod_row["author_id"])) + + +def _mention_or_unknown(author) -> str: + return f"de {author.mention}" if author else "(el autor ya no está disponible)" + + class RejectModal(discord.ui.Modal, title="Rechazar Mensaje"): reason = discord.ui.TextInput( label="Razón del rechazo", @@ -58,45 +73,110 @@ class RejectModal(discord.ui.Modal, title="Rechazar Mensaje"): required=True ) - def __init__(self, author: discord.Member, cog, message_id: int): + def __init__(self, cog, message_id: int, author: Optional[discord.abc.User] = None): super().__init__() - self.author = author self.cog = cog self.message_id = message_id + self.author = author async def on_submit(self, interaction: discord.Interaction): mod = interaction.user await interaction.response.send_message( - f"{mod.mention} rechazó el mensaje de {self.author.mention}.\n" + f"{mod.mention} rechazó el mensaje {_mention_or_unknown(self.author)}.\n" f"Razón: {self.reason.value}", ephemeral=True ) await self.cog._rechazar_mensaje(interaction, self.message_id, self.reason.value) -class ApproveRejectView(discord.ui.View): - def __init__(self, author: discord.Member, cog, message_id: int): - super().__init__(timeout=None) - self.author = author - self.cog = cog +class ApproveButton( + discord.ui.DynamicItem[discord.ui.Button], + template=r"moderacion:aprobar:(?P[0-9]+)", +): + """A button whose target message is encoded in its ``custom_id`` + instead of Python closure/constructor state. + + ``discord.ui.View(timeout=None)`` alone (the previous approach) only + keeps a button from visually expiring - it does NOT make it survive a + bot restart, since the view instance holding ``author``/``message_id`` + in memory is gone once the process restarts, and Discord has nothing to + route the click to. This DynamicItem (paired with + ``bot.add_dynamic_items()`` in bot.py) is reconstructed from the + ``custom_id`` alone via ``from_custom_id``, so a click always works + regardless of how long ago the message was sent or whether the bot + restarted since. + """ + + def __init__(self, message_id: int): + super().__init__( + discord.ui.Button( + label="Aprobar", + style=discord.ButtonStyle.success, + custom_id=f"moderacion:aprobar:{message_id}", + ) + ) self.message_id = message_id - @discord.ui.button(label="Aprobar", style=discord.ButtonStyle.success) - async def approve_button(self, interaction: discord.Interaction, button: discord.ui.Button): + @classmethod + async def from_custom_id(cls, interaction, item, match, /): + return cls(int(match["message_id"])) + + async def callback(self, interaction: discord.Interaction): + cog = interaction.client.get_cog("Moderacion") + if cog is None: + return mod = interaction.user + author = _pending_author(cog, self.message_id) await interaction.response.send_message( - f"{mod.mention} aprobó el mensaje de {self.author.mention}.", - ephemeral=True + f"{mod.mention} aprobó el mensaje {_mention_or_unknown(author)}.", + ephemeral=True, + ) + await cog._aceptar_mensaje(interaction, self.message_id) + + +class RejectButton( + discord.ui.DynamicItem[discord.ui.Button], + template=r"moderacion:rechazar:(?P[0-9]+)", +): + """Persistent counterpart to ``ApproveButton`` above - see its + docstring.""" + + def __init__(self, message_id: int): + super().__init__( + discord.ui.Button( + label="Rechazar", + style=discord.ButtonStyle.danger, + custom_id=f"moderacion:rechazar:{message_id}", + ) ) - await self.cog._aceptar_mensaje(interaction, self.message_id) + self.message_id = message_id + + @classmethod + async def from_custom_id(cls, interaction, item, match, /): + return cls(int(match["message_id"])) - @discord.ui.button(label="Rechazar", style=discord.ButtonStyle.danger) - async def reject_button(self, interaction: discord.Interaction, button: discord.ui.Button): + async def callback(self, interaction: discord.Interaction): + cog = interaction.client.get_cog("Moderacion") + if cog is None: + return + author = _pending_author(cog, self.message_id) await interaction.response.send_modal( - RejectModal(author=self.author, cog=self.cog, message_id=self.message_id) + RejectModal(cog=cog, message_id=self.message_id, author=author) ) +class ApproveRejectView(discord.ui.View): + """Sent once, right after a submission - built from the persistent + DynamicItem buttons above, so it keeps responding to clicks even across + a bot restart (as long as ``bot.add_dynamic_items()`` ran on startup - + see bot.py).""" + + def __init__(self, message_id: int): + super().__init__(timeout=None) + self.add_item(ApproveButton(message_id)) + self.add_item(RejectButton(message_id)) + + class Moderacion(commands.Cog): def __init__(self, bot): self.bot = bot @@ -164,6 +244,18 @@ async def _get_validated_post( message_dec = _decode_message(mod_row["message"]) author = self.bot.get_user(int(mod_row["author_id"])) + if author is None: + # Same condition get_mod_pending() silently skips - here we can't + # silently skip it (the moderator is actively trying to act on + # it), so make it an explicit, visible failure instead of + # continuing with author=None and crashing later on + # author.mention with no trace of why. + logger.warning("El author '%s' ya no existe en el server.", mod_row["author_id"]) + await channel_mod.send( + f"El autor del mensaje `{post_id}` (ID `{mod_row['author_id']}`) ya no está " + "en el servidor; no se puede procesar." + ) + return None return ValidatedPost( post_id=post_id, @@ -259,7 +351,7 @@ async def on_message(self, message): reply_msg = await ch_sub.send(embed=embed) embed = get_message_to_moderate(message) - view = ApproveRejectView(message.author, cog=self, message_id=message.id) + view = ApproveRejectView(message_id=message.id) await ch_mod.send(embed=embed, view=view) await asyncio.sleep(3) @@ -272,16 +364,35 @@ async def _aceptar_mensaje(self, ctx, message_id: Optional[int] = None): if vp is None: return + # Post to the destination channel *first*, and only log the accept / + # drop it from bot.data_mod once that actually succeeds. Doing this + # the other way around (as before) meant a failure here - message + # over Discord's 2000-char limit, missing permissions in the + # destination channel, a transient network hiccup - silently lost + # the post: it had already been logged "aceptado" and removed from + # the pending queue, so nothing showed it never actually went out. + try: + sent_message = await vp.ch_main.send( + f"> [Enviado por {vp.author.mention}]\n{vp.message_dec}" + ) + except Exception: + logger.exception( + "Fallo al enviar el mensaje aceptado %s a %s", vp.post_id, vp.ch_main + ) + await vp.ch_mod.send( + f"\N{WARNING SIGN} No se pudo enviar el mensaje `{vp.post_id}` a " + f"{vp.ch_main.mention}. Sigue pendiente - revisa bot.log e intenta de nuevo." + ) + return + moderator = self._resolve_author(ctx) self._log_action("aceptar", vp.mod_row, vp.post_id, moderator) del self.bot.data_mod[vp.post_id] - # Send to the destination channel first so the confirmation below can - # link to the message that was actually posted there, instead of + # Link to the message that was actually posted there, instead of # guessing at a URL (the old code built the link from self._msg_id - # the *original submission's* id in a different channel entirely - # before the message below even existed). - sent_message = await vp.ch_main.send(f"> [Enviado por {vp.author.mention}]\n{vp.message_dec}") await vp.ch_mod.send( f"{aceptar_emoji} Mensaje `{vp.post_id}` aceptado, " f"enviado al canal {vp.ch_main.mention}\nVer en {sent_message.jump_url}" @@ -304,10 +415,6 @@ async def _rechazar_mensaje( _post = ctx.message.content.replace("%rechazar", "").strip().split() reason = " ".join(_post[1:]) # everything after the ID - moderator = self._resolve_author(ctx) - self._log_action("rechazar", vp.mod_row, vp.post_id, moderator, reason or "") - del self.bot.data_mod[vp.post_id] - embed = discord.Embed( title="Mensaje rechazado", description=f"{vp.author.mention} tu mensaje necesita atención.", @@ -320,11 +427,29 @@ async def _rechazar_mensaje( ) embed.add_field(name="Mensaje original", value=vp.message_dec, inline=False) + # Same reasoning as _aceptar_mensaje: notify the submitter *first*, + # only log the reject / drop it from bot.data_mod once that actually + # goes through, so a failure here doesn't quietly lose the item. + try: + await vp.ch_sub.send(embed=embed) + except Exception: + logger.exception( + "Fallo al notificar el rechazo del mensaje %s a %s", vp.post_id, vp.ch_sub + ) + await vp.ch_mod.send( + f"\N{WARNING SIGN} No se pudo notificar el rechazo del mensaje `{vp.post_id}` " + f"a {vp.ch_sub.mention}. Sigue pendiente - revisa bot.log e intenta de nuevo." + ) + return + + moderator = self._resolve_author(ctx) + self._log_action("rechazar", vp.mod_row, vp.post_id, moderator, reason or "") + del self.bot.data_mod[vp.post_id] + await vp.ch_mod.send( f"{rechazar_emoji} Mensaje `{vp.post_id}` rechazado, " f"enviada respuesta a {vp.ch_mod.mention}" ) - await vp.ch_sub.send(embed=embed) @commands.command(name="rechazar", help="Comando para rechazar mensajes en moderación") @commands.has_role(config.MOD_ROLE) diff --git a/tests/test_moderacion.py b/tests/test_moderacion.py index 71e9f8f..d82b92a 100644 --- a/tests/test_moderacion.py +++ b/tests/test_moderacion.py @@ -3,7 +3,14 @@ import pytest -from comandos.moderacion import _decode_message, _encode_message +from comandos.moderacion import ( + ApproveButton, + RejectButton, + RejectModal, + _decode_message, + _encode_message, + _pending_author, +) from tests.factories import ( encode_for_mod_row, encode_for_mod_row_legacy, @@ -169,6 +176,24 @@ async def test_bot_author_returns_none(self, moderacion_cog, moderacion_channels assert await moderacion_cog._get_validated_post(ctx, None, "%aceptar") is None + async def test_unknown_author_reports_error_and_returns_none( + self, moderacion_cog, moderacion_channels + ): + """Regression test: this used to fall through with author=None and + crash later on author.mention with no clear explanation - now it's + an explicit, visible failure instead, mirroring get_mod_pending()'s + handling of the same condition.""" + add_pending_row(moderacion_cog, post_id=1, author_id=404) + # bot.get_user(404) resolves to None - author has left the server. + ctx = make_ctx(channel=moderacion_channels["mod"], content="%aceptar 1") + + vp = await moderacion_cog._get_validated_post(ctx, None, "%aceptar") + + assert vp is None + moderacion_channels["mod"].send.assert_awaited_once() + (msg,), _ = moderacion_channels["mod"].send.call_args + assert "404" in msg + # --------------------------------------------------------------------------- # _log_action / log_on_message @@ -306,6 +331,30 @@ async def test_unknown_post_id_does_not_touch_channels(self, moderacion_cog, mod moderacion_channels["main"].send.assert_not_awaited() + async def test_send_failure_keeps_the_row_pending_and_reports_the_error( + self, moderacion_cog, moderacion_channels, isolated_logs + ): + """Regression test: sending to ch_main used to happen *after* + logging the accept and dropping the row from bot.data_mod, so a + failure there (message too long, missing permissions, a transient + network error, ...) silently lost the post - it was already logged + "aceptado" and gone from the pending queue with no visible error + anywhere. Now the row must survive a send failure so it can be + retried, and the failure must be visible in the mod channel.""" + add_pending_row(moderacion_cog, post_id=1, author_id=99, content="contenido") + moderacion_cog.bot.users_by_id[99] = make_member(id=99, name="remitente") + moderacion_channels["main"].send = AsyncMock(side_effect=RuntimeError("boom")) + + ctx = make_ctx(channel=moderacion_channels["mod"], content="%aceptar 1") + await moderacion_cog._aceptar_mensaje(ctx) + + assert "1" in moderacion_cog.bot.data_mod # still pending, not lost + moderacion_channels["mod"].send.assert_awaited_once() + (msg,), _ = moderacion_channels["mod"].send.call_args + assert "No se pudo enviar" in msg + with isolated_logs.log_accepted_file.open() as f: + assert f.read().strip() == "" # nothing falsely logged as accepted + class TestRechazarMensaje: async def test_removes_pending_row_and_notifies_with_reason( @@ -339,6 +388,26 @@ async def test_interaction_path_uses_the_provided_reason( _, kwargs = moderacion_channels["sub"].send.call_args assert "motivo modal" in kwargs["embed"].fields[0].value + async def test_notify_failure_keeps_the_row_pending_and_reports_the_error( + self, moderacion_cog, moderacion_channels, isolated_logs + ): + """Same reordering fix as the aceptar-side regression test above: + a failure notifying the submitter must not lose the row or make the + mod channel claim success anyway.""" + add_pending_row(moderacion_cog, post_id=4, author_id=99, content="contenido") + moderacion_cog.bot.users_by_id[99] = make_member(id=99, name="remitente") + moderacion_channels["sub"].send = AsyncMock(side_effect=RuntimeError("boom")) + + ctx = make_ctx(channel=moderacion_channels["mod"], content="%rechazar 4 motivo") + await moderacion_cog._rechazar_mensaje(ctx) + + assert "4" in moderacion_cog.bot.data_mod # still pending, not lost + moderacion_channels["mod"].send.assert_awaited_once() + (msg,), _ = moderacion_channels["mod"].send.call_args + assert "No se pudo notificar" in msg + with isolated_logs.log_rejected_file.open() as f: + assert f.read().strip() == "" # nothing falsely logged as rejected + # --------------------------------------------------------------------------- # on_message listener @@ -385,3 +454,128 @@ async def test_submission_gets_logged_and_forwarded_to_mod_channel( assert len(moderacion_cog.bot.data_mod) == before + 1 moderacion_channels["sub"].send.assert_awaited_once() moderacion_channels["mod"].send.assert_awaited_once() + + +# --------------------------------------------------------------------------- +# Persistent Aprobar/Rechazar buttons (discord.ui.DynamicItem) +# +# These reconstruct themselves from custom_id alone (from_custom_id) and +# resolve the cog via interaction.client.get_cog(...) instead of a Python +# closure, which is exactly what makes them keep working after a bot +# restart - see ApproveButton's docstring in comandos/moderacion.py. +# --------------------------------------------------------------------------- +def _match_custom_id(button_cls, message_id): + """A real re.Match for button_cls's template, the same kind + discord.py hands to from_custom_id() when routing an interaction.""" + custom_id = f"moderacion:{'aprobar' if button_cls is ApproveButton else 'rechazar'}:{message_id}" + return button_cls.__discord_ui_compiled_template__.match(custom_id) + + +class TestPendingAuthor: + def test_resolves_the_author_of_a_pending_row(self, moderacion_cog): + add_pending_row(moderacion_cog, post_id=1, author_id=99) + moderacion_cog.bot.users_by_id[99] = make_member(id=99, name="remitente") + + assert _pending_author(moderacion_cog, 1).id == 99 + + def test_unknown_message_id_returns_none(self, moderacion_cog): + assert _pending_author(moderacion_cog, 404) is None + + +class TestApproveButton: + async def test_from_custom_id_round_trips_the_message_id(self): + match = _match_custom_id(ApproveButton, 12345) + + button = await ApproveButton.from_custom_id(None, None, match) + + assert button.message_id == 12345 + assert button.item.custom_id == "moderacion:aprobar:12345" + + async def test_callback_resolves_cog_via_interaction_client_and_delegates( + self, moderacion_cog, moderacion_channels + ): + add_pending_row(moderacion_cog, post_id=1, author_id=99) + moderacion_cog.bot.users_by_id[99] = make_member(id=99, name="remitente") + moderacion_cog._aceptar_mensaje = AsyncMock() + interaction = make_interaction(channel=moderacion_channels["mod"]) + interaction.client = SimpleNamespace(get_cog=lambda name: moderacion_cog) + + button = ApproveButton(message_id=1) + await button.callback(interaction) + + interaction.response.send_message.assert_awaited_once() + (msg,), _ = interaction.response.send_message.call_args + assert "<@99>" in msg # make_member()'s fake mention + moderacion_cog._aceptar_mensaje.assert_awaited_once_with(interaction, 1) + + async def test_callback_with_no_longer_pending_row_still_delegates( + self, moderacion_cog, moderacion_channels + ): + """The row may already be gone (already handled, or from a message + old enough the bot restarted since) - _pending_author then returns + None, and the button falls back to a generic message instead of + crashing on author.mention.""" + moderacion_cog._aceptar_mensaje = AsyncMock() + interaction = make_interaction(channel=moderacion_channels["mod"]) + interaction.client = SimpleNamespace(get_cog=lambda name: moderacion_cog) + + button = ApproveButton(message_id=999) + await button.callback(interaction) + + (msg,), _ = interaction.response.send_message.call_args + assert "ya no está disponible" in msg + moderacion_cog._aceptar_mensaje.assert_awaited_once_with(interaction, 999) + + async def test_callback_without_a_registered_cog_does_nothing(self): + """If the bot somehow has no Moderacion cog loaded, don't crash - + just skip (nothing to delegate to).""" + interaction = make_interaction() + interaction.client = SimpleNamespace(get_cog=lambda name: None) + + await ApproveButton(message_id=1).callback(interaction) + + interaction.response.send_message.assert_not_awaited() + + +class TestRejectButton: + async def test_from_custom_id_round_trips_the_message_id(self): + match = _match_custom_id(RejectButton, 777) + + button = await RejectButton.from_custom_id(None, None, match) + + assert button.message_id == 777 + + async def test_callback_opens_a_modal_for_the_resolved_author( + self, moderacion_cog, moderacion_channels + ): + add_pending_row(moderacion_cog, post_id=2, author_id=99) + moderacion_cog.bot.users_by_id[99] = make_member(id=99, name="remitente") + interaction = make_interaction(channel=moderacion_channels["mod"]) + interaction.client = SimpleNamespace(get_cog=lambda name: moderacion_cog) + + await RejectButton(message_id=2).callback(interaction) + + interaction.response.send_modal.assert_awaited_once() + (modal,), _ = interaction.response.send_modal.call_args + assert isinstance(modal, RejectModal) + assert modal.message_id == 2 + assert modal.author.id == 99 + + +class TestRejectModal: + async def test_on_submit_delegates_with_the_entered_reason(self, moderacion_cog): + cog = moderacion_cog + cog._rechazar_mensaje = AsyncMock() + author = make_member(id=99, name="remitente") + modal = RejectModal(cog=cog, message_id=5, author=author) + modal.reason._value = "no cumple los requisitos" + interaction = make_interaction() + + await modal.on_submit(interaction) + + interaction.response.send_message.assert_awaited_once() + (msg,), _ = interaction.response.send_message.call_args + assert "<@99>" in msg + cog._rechazar_mensaje.assert_awaited_once_with( + interaction, 5, "no cumple los requisitos" + )