Browse Source

Commandes et /memoire : message en cas d'erreur, boutons grisés à l'expiration

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Johnounet 2 days ago
parent
commit
0e7b7250fb
5 changed files with 134 additions and 8 deletions
  1. 5 1
      bot/discord_bot.py
  2. 51 6
      bot/memory_views.py
  3. 6 0
      changelog.txt
  4. 11 0
      tests/test_home_guild.py
  5. 61 1
      tests/test_memory.py

+ 5 - 1
bot/discord_bot.py

@@ -24,7 +24,7 @@ from .history import IMAGE_MARKER, ConversationHistory, render_line
 from .images import ImageError, prepare_image
 from .images import ImageError, prepare_image
 from .members import MemberRegistry, with_username
 from .members import MemberRegistry, with_username
 from .memory import MemoryStore
 from .memory import MemoryStore
-from .memory_views import MemoryCard, member_target, see_user, server_target
+from .memory_views import MemoryCard, member_target, reply_error, see_user, server_target
 from .models import is_known
 from .models import is_known
 from .pdf_files import PDF_EXTENSIONS, PDF_MAX_BYTES, PdfError, prepare_pdf
 from .pdf_files import PDF_EXTENSIONS, PDF_MAX_BYTES, PdfError, prepare_pdf
 from .presence import PresenceManager, Status
 from .presence import PresenceManager, Status
@@ -125,6 +125,10 @@ class HomeTree(app_commands.CommandTree):
                                                     ephemeral=True)
                                                     ephemeral=True)
         return False
         return False
 
 
+    async def on_error(self, interaction: discord.Interaction, error: app_commands.AppCommandError) -> None:
+        logger.error("Erreur pendant /%s", (interaction.data or {}).get("name", "?"), exc_info=error)
+        await reply_error(interaction)
+
 
 
 class ChatBot(commands.Bot):
 class ChatBot(commands.Bot):
     def __init__(self, settings: Settings):
     def __init__(self, settings: Settings):

+ 51 - 6
bot/memory_views.py

@@ -25,12 +25,28 @@ EMBED_MAX = 4000
 EMBEDS_TOTAL_MAX = 5500
 EMBEDS_TOTAL_MAX = 5500
 # Durée pendant laquelle les boutons répondent (ensuite, il suffit de relancer la commande).
 # Durée pendant laquelle les boutons répondent (ensuite, il suffit de relancer la commande).
 VIEW_TIMEOUT = 600
 VIEW_TIMEOUT = 600
+# Durée pendant laquelle la fenêtre « Modifier » peut être validée : Discord la laisse ouverte sans
+# limite, mais discord.py l'oublie ensuite et ignore sa validation. Assez pour réécrire le fichier général.
+MODAL_TIMEOUT = 3600
 # Sous la fiche d'un membre, quand il la consulte lui-même.
 # Sous la fiche d'un membre, quand il la consulte lui-même.
 ADMINS_NOTE = "Les administrateurs du serveur peuvent aussi voir et corriger ta fiche."
 ADMINS_NOTE = "Les administrateurs du serveur peuvent aussi voir et corriger ta fiche."
+ERROR_MESSAGE = "Une erreur est survenue. Si ça se reproduit, préviens l'administrateur du bot."
 
 
 Callback = Callable[[discord.Interaction], Awaitable[None]]
 Callback = Callable[[discord.Interaction], Awaitable[None]]
 
 
 
 
+async def reply_error(interaction: discord.Interaction) -> None:
+    """Prévient la personne qu'une commande, un bouton ou une fenêtre a échoué (le détail est dans le log),
+    au lieu du seul « Échec de l'interaction » de Discord."""
+    try:
+        if interaction.response.is_done():
+            await interaction.followup.send(ERROR_MESSAGE, ephemeral=True)
+        else:
+            await interaction.response.send_message(ERROR_MESSAGE, ephemeral=True)
+    except discord.HTTPException as exc:
+        logger.warning("Impossible de signaler l'erreur à %s : %s", interaction.user, exc)
+
+
 async def see_user(registry: MemberRegistry, user: discord.User | discord.Member) -> None:
 async def see_user(registry: MemberRegistry, user: discord.User | discord.Member) -> None:
     """Ajoute un utilisateur Discord au registre, ou met ses noms à jour."""
     """Ajoute un utilisateur Discord au registre, ou met ses noms à jour."""
     # En message privé, l'auteur n'est pas un membre du serveur : son pseudo n'est pas connu.
     # En message privé, l'auteur n'est pas un membre du serveur : son pseudo n'est pas connu.
@@ -55,6 +71,7 @@ class MemoryCard:
         self.target = target
         self.target = target
         self.owner_id = owner_id
         self.owner_id = owner_id
         self.bot_name = bot_name
         self.bot_name = bot_name
+        self._view: OwnerView | None = None  # boutons affichés en ce moment
 
 
     # ---------------------------------------------------------------- état
     # ---------------------------------------------------------------- état
 
 
@@ -121,11 +138,22 @@ class MemoryCard:
         return view
         return view
 
 
     async def send(self, interaction: discord.Interaction) -> None:
     async def send(self, interaction: discord.Interaction) -> None:
-        await interaction.response.send_message(embeds=self.embeds(), view=self.view(), ephemeral=True)
+        await self.show(interaction, self.view(), self.embeds(), new=True)
 
 
     async def refresh(self, interaction: discord.Interaction, note: str | None = None) -> None:
     async def refresh(self, interaction: discord.Interaction, note: str | None = None) -> None:
         """Réaffiche le fichier à jour dans le même message, avec une courte phrase au-dessus."""
         """Réaffiche le fichier à jour dans le même message, avec une courte phrase au-dessus."""
-        await interaction.response.edit_message(content=note, embeds=self.embeds(), view=self.view())
+        await self.show(interaction, self.view(), self.embeds(), note)
+
+    async def show(self, interaction: discord.Interaction, view: OwnerView, embeds: list[discord.Embed],
+                   note: str | None = None, new: bool = False) -> None:
+        """Affiche ces boutons dans un nouveau message éphémère (new), ou à la place des précédents."""
+        if self._view is not None:
+            self._view.stop()  # sans quoi son expiration griserait les nouveaux boutons
+        self._view, view.interaction = view, interaction
+        if new:
+            await interaction.response.send_message(embeds=embeds, view=view, ephemeral=True)
+        else:
+            await interaction.response.edit_message(content=note, embeds=embeds, view=view)
 
 
     # -------------------------------------------------------------- actions
     # -------------------------------------------------------------- actions
 
 
@@ -203,10 +231,28 @@ class OwnerView(discord.ui.View):
     def __init__(self, owner_id: int):
     def __init__(self, owner_id: int):
         super().__init__(timeout=VIEW_TIMEOUT)
         super().__init__(timeout=VIEW_TIMEOUT)
         self.owner_id = owner_id
         self.owner_id = owner_id
+        self.interaction: discord.Interaction | None = None  # celle qui a affiché ces boutons (voir show)
 
 
     async def interaction_check(self, interaction: discord.Interaction) -> bool:
     async def interaction_check(self, interaction: discord.Interaction) -> bool:
         return interaction.user.id == self.owner_id
         return interaction.user.id == self.owner_id
 
 
+    async def on_timeout(self) -> None:
+        """Boutons grisés : ils ne répondent plus. Le message éphémère reste modifiable 15 minutes après
+        l'interaction qui l'a affiché, plus que VIEW_TIMEOUT."""
+        if self.interaction is None:
+            return
+        for item in self.children:
+            item.disabled = True
+        try:
+            await self.interaction.edit_original_response(
+                content="Ces boutons ont expiré : relance la commande pour continuer.", view=self)
+        except discord.HTTPException:
+            pass  # message fermé par la personne, ou trop ancien
+
+    async def on_error(self, interaction: discord.Interaction, error: Exception, item: discord.ui.Item) -> None:
+        logger.exception("Erreur sur le bouton « %s »", getattr(item, "label", "?"), exc_info=error)
+        await reply_error(interaction)
+
 
 
 def add_button(view: discord.ui.View, label: str, emoji: str, style: discord.ButtonStyle,
 def add_button(view: discord.ui.View, label: str, emoji: str, style: discord.ButtonStyle,
                callback: Callback, disabled: bool = False) -> None:
                callback: Callback, disabled: bool = False) -> None:
@@ -221,7 +267,7 @@ async def confirm(interaction: discord.Interaction, card: MemoryCard, question:
     add_button(view, "Confirmer", "⚠️", discord.ButtonStyle.danger, action)
     add_button(view, "Confirmer", "⚠️", discord.ButtonStyle.danger, action)
     add_button(view, "Annuler", "↩️", discord.ButtonStyle.secondary, card.refresh)
     add_button(view, "Annuler", "↩️", discord.ButtonStyle.secondary, card.refresh)
     embed = discord.Embed(description=question, color=discord.Color.orange())
     embed = discord.Embed(description=question, color=discord.Color.orange())
-    await interaction.response.edit_message(content=None, embeds=[embed], view=view)
+    await card.show(interaction, view, [embed])
 
 
 
 
 class EditModal(discord.ui.Modal):
 class EditModal(discord.ui.Modal):
@@ -231,7 +277,7 @@ class EditModal(discord.ui.Modal):
     def __init__(self, card: MemoryCard):
     def __init__(self, card: MemoryCard):
         title = "Modifier ta fiche" if card.target.is_self else (
         title = "Modifier ta fiche" if card.target.is_self else (
             "Modifier le fichier général" if card.target.member_id is None else f"Fiche de {card.target.name}")
             "Modifier le fichier général" if card.target.member_id is None else f"Fiche de {card.target.name}")
-        super().__init__(title=title[:45], timeout=VIEW_TIMEOUT)
+        super().__init__(title=title[:45], timeout=MODAL_TIMEOUT)
         self.card = card
         self.card = card
         limit = card.store.limit(card.target.file)
         limit = card.store.limit(card.target.file)
         self.original = card.store.read(card.target.file)  # comparé à l'enregistrement (voir overwrite)
         self.original = card.store.read(card.target.file)  # comparé à l'enregistrement (voir overwrite)
@@ -261,8 +307,7 @@ class EditModal(discord.ui.Modal):
 
 
     async def on_error(self, interaction: discord.Interaction, error: Exception) -> None:
     async def on_error(self, interaction: discord.Interaction, error: Exception) -> None:
         logger.exception("Erreur pendant la modification de %s", self.card.target.file, exc_info=error)
         logger.exception("Erreur pendant la modification de %s", self.card.target.file, exc_info=error)
-        if not interaction.response.is_done():
-            await interaction.response.send_message("La modification a échoué, regarde les logs.", ephemeral=True)
+        await reply_error(interaction)
 
 
 
 
 def input_sizes(limit: int) -> list[int]:
 def input_sizes(limit: int) -> list[int]:

+ 6 - 0
changelog.txt

@@ -132,6 +132,12 @@ Corrections
   fenêtre ne s'ouvrait pas (« Échec de l'interaction »). Au-delà de
   fenêtre ne s'ouvrait pas (« Échec de l'interaction »). Au-delà de
   20000 caractères, trop pour une fenêtre Discord, son contenu est
   20000 caractères, trop pour une fenêtre Discord, son contenu est
   envoyé en fichier joint.
   envoyé en fichier joint.
+- Commandes et boutons : une erreur affiche « Une erreur est survenue »
+  au lieu du seul « Échec de l'interaction » de Discord. Les boutons de
+  /memoire se grisent au bout de 10 minutes, avec un message qui invite
+  à relancer la commande, au lieu de rester affichés sans répondre. La
+  fenêtre « Modifier » peut être validée pendant une heure, au lieu de
+  10 minutes.
 - Mémoire : un membre ne peut plus prendre le nom Discord d'un autre
 - Mémoire : un membre ne peut plus prendre le nom Discord d'un autre
   (pseudo, nom d'utilisateur ou ancien pseudo) comme surnom dans sa
   (pseudo, nom d'utilisateur ou ancien pseudo) comme surnom dans sa
   fiche. Ce surnom est ignoré dans la liste des membres : Rika ne note
   fiche. Ce surnom est ignoré dans la liste des membres : Rika ne note

+ 11 - 0
tests/test_home_guild.py

@@ -4,8 +4,10 @@ import unittest
 from unittest import mock
 from unittest import mock
 
 
 import discord
 import discord
+from discord import app_commands
 
 
 from bot.discord_bot import ChatBot, HomeTree
 from bot.discord_bot import ChatBot, HomeTree
+from bot.memory_views import ERROR_MESSAGE
 
 
 HOME, OTHER = 1000, 2000
 HOME, OTHER = 1000, 2000
 CHANNEL_ID = 10
 CHANNEL_ID = 10
@@ -52,6 +54,15 @@ class CommandsTest(unittest.IsolatedAsyncioTestCase):
     async def test_refused_while_channel_unknown(self):
     async def test_refused_while_channel_unknown(self):
         self.assertFalse(await self.check(fake_interaction(HOME), channel_found=False))
         self.assertFalse(await self.check(fake_interaction(HOME), channel_found=False))
 
 
+    async def test_command_error_reported(self):
+        # Une exception dans une commande : un message à la personne, plus le seul « Échec de l'interaction ».
+        interaction = fake_interaction(HOME)
+        interaction.response.is_done.return_value = False
+        tree = object.__new__(HomeTree)
+        with self.assertLogs("chatbot.discord", "ERROR"):
+            await tree.on_error(interaction, app_commands.AppCommandError("panne"))
+        self.assertEqual(interaction.response.send_message.call_args.args[0], ERROR_MESSAGE)
+
 
 
 class MessagesTest(unittest.IsolatedAsyncioTestCase):
 class MessagesTest(unittest.IsolatedAsyncioTestCase):
     async def receive(self, guild: mock.Mock | None, channel_id: int = CHANNEL_ID) -> mock.Mock:
     async def receive(self, guild: mock.Mock | None, channel_id: int = CHANNEL_ID) -> mock.Mock:

+ 61 - 1
tests/test_memory.py

@@ -7,9 +7,12 @@ import unittest
 from pathlib import Path
 from pathlib import Path
 from unittest import mock
 from unittest import mock
 
 
+import discord
+
 from bot.members import MemberRegistry, with_username
 from bot.members import MemberRegistry, with_username
 from bot.discord_bot import memory_command
 from bot.discord_bot import memory_command
-from bot.memory_views import ADMINS_NOTE, EditModal, MemoryCard, Target, input_sizes, split_for_inputs
+from bot.memory_views import (ADMINS_NOTE, ERROR_MESSAGE, MODAL_TIMEOUT, EditModal, MemoryCard, Target, input_sizes,
+                              reply_error, split_for_inputs)
 from bot.memory import (MAX_ATTACHED_NOTES, MAX_FORGOTTEN_LINES, MAX_NICKNAME_LENGTH, MAX_NICKNAMES,
 from bot.memory import (MAX_ATTACHED_NOTES, MAX_FORGOTTEN_LINES, MAX_NICKNAME_LENGTH, MAX_NICKNAMES,
                         SERVER_FILE, MemoryStore)
                         SERVER_FILE, MemoryStore)
 
 
@@ -626,6 +629,63 @@ class TooLongFileTest(MemoryTestCase):
         self.assertEqual(len(message.kwargs["file"].fp.getvalue()), 25139)
         self.assertEqual(len(message.kwargs["file"].fp.getvalue()), 25139)
 
 
 
 
+class ExpiryAndErrorsTest(MemoryTestCase):
+    """Boutons grisés à l'expiration, message en cas d'erreur, fenêtre « Modifier » plus longue."""
+
+    @staticmethod
+    def interaction(done: bool = False) -> mock.Mock:
+        interaction = mock.Mock()
+        interaction.response.is_done.return_value = done
+        interaction.response.send_message = mock.AsyncMock()
+        interaction.response.edit_message = mock.AsyncMock()
+        interaction.followup.send = mock.AsyncMock()
+        interaction.edit_original_response = mock.AsyncMock()
+        return interaction
+
+    def card(self) -> MemoryCard:
+        return MemoryCard(self.store, Target(f"membres/{MEMBER_ID}.md", MEMBER_ID, "Bob", is_self=True),
+                          owner_id=MEMBER_ID, bot_name="Rika")
+
+    async def test_buttons_disabled_on_timeout(self):
+        card, interaction = self.card(), self.interaction()
+        await card.send(interaction)
+        view = interaction.response.send_message.call_args.kwargs["view"]
+        await view.on_timeout()
+        edit = interaction.edit_original_response.call_args.kwargs
+        self.assertIn("expiré", edit["content"])
+        self.assertTrue(all(item.disabled for item in edit["view"].children))
+
+    async def test_replaced_buttons_stopped(self):
+        # Sans cela, l'expiration des anciens boutons griserait ceux qui les ont remplacés.
+        card = self.card()
+        await card.send(self.interaction())
+        first = card._view
+        await card.refresh(self.interaction())
+        self.assertTrue(first.is_finished())
+        self.assertFalse(card._view.is_finished())
+
+    async def test_timeout_after_message_closed(self):
+        card, interaction = self.card(), self.interaction()
+        await card.send(interaction)
+        interaction.edit_original_response.side_effect = discord.NotFound(mock.Mock(status=404), "inconnu")
+        await card._view.on_timeout()  # aucune exception
+
+    async def test_button_error_reported(self):
+        card, interaction = self.card(), self.interaction()
+        await card.send(self.interaction())
+        with self.assertLogs("chatbot.memory", "ERROR"):
+            await card._view.on_error(interaction, RuntimeError("panne"), card._view.children[0])
+        self.assertEqual(interaction.response.send_message.call_args.args[0], ERROR_MESSAGE)
+
+    async def test_error_after_response_uses_followup(self):
+        interaction = self.interaction(done=True)
+        await reply_error(interaction)
+        interaction.followup.send.assert_awaited_once_with(ERROR_MESSAGE, ephemeral=True)
+
+    async def test_modal_lasts_longer(self):
+        self.assertEqual(EditModal(self.card()).timeout, MODAL_TIMEOUT)
+
+
 class AdminAccessTest(MemoryTestCase):
 class AdminAccessTest(MemoryTestCase):
     """Les membres savent que les administrateurs voient leur fiche, et chaque consultation est tracée."""
     """Les membres savent que les administrateurs voient leur fiche, et chaque consultation est tracée."""