Improve animal generation command UX

Author: NMertsch

src/europython_discord/animals/cog.py

@@ -6,6 +6,7 @@
 
 import discord
 from discord.ext import commands
+from discord.utils import get as discord_get
 
 from europython_discord.animals import providers
 from europython_discord.animals.config import AnimalsConfig
@@ -28,17 +29,31 @@ def __init__(
         self._providers = providers_by_animal
         self._rate_limiter = rate_limiter
 
-        for animal in self._providers:
-            self._bot.add_command(self._create_animal_command(animal))
-
         _logger.info("Cog 'Animals' has been initialized")
 
+    @commands.Cog.listener()
+    async def on_ready(self) -> None:
+        # (re-) register commands on each connected guild
+        for guild in self._bot.guilds:
+            self._bot.tree.clear_commands(guild=guild)
+            for animal in self._providers:
+                self._bot.tree.add_command(self._create_animal_command(animal), guild=guild)
+            synced_commands = await self._bot.tree.sync(guild=guild)
+            _logger.info(
+                "Synced %d application command(s) to guild %s", len(synced_commands), guild.id
+            )
+
     async def post_animal_picture(self, animal: str, ctx: commands.Context) -> None:
+        # send typing indicator
+        await ctx.defer(ephemeral=False)
+
+        # check if command shall be executed
         if ctx.channel.name != self._channel_name:
+            channel = discord_get(ctx.guild.channels, name=self._channel_name)
+            await ctx.send(f"This command can only be used in {channel.mention}.")
             return
         if self._rate_limiter.is_rate_limited(ctx.author.id):
-            return
-        if animal not in self._providers:
+            await ctx.send("You are being rate limited. Please try again later.")
             return
 
         provider = random.choice(self._providers[animal])  # noqa: S311 suspicious-non-cryptographic-random-usage
@@ -51,7 +66,13 @@ async def post_animal_picture(self, animal: str, ctx: commands.Context) -> None:
 
         if image is None:
             _logger.error("Failed to fetch %s pictures", animal)
-            await ctx.send(f"Failed to fetch {animal} picture.")
+            await ctx.send(
+                (
+                    f"Failed to fetch {animal} picture. "
+                    f"If this happens repeatedly, please report it."
+                ),
+                ephemeral=True,
+            )
             return
 
         embed = discord.Embed()
@@ -60,9 +81,10 @@ async def post_animal_picture(self, animal: str, ctx: commands.Context) -> None:
         self._rate_limiter.register_usage(ctx.author.id)
         await ctx.send(embed=embed)
 
-    def _create_animal_command(self, animal: str) -> commands.HybridCommand:
-        @commands.hybrid_command(name=animal, description=f"Get a random {animal} picture")
-        async def callback(ctx: commands.Context) -> None:
+    def _create_animal_command(self, animal: str) -> discord.app_commands.Command:
+        @discord.app_commands.command(name=animal, description=f"Get a random {animal} picture")
+        async def callback(interaction: discord.Interaction) -> None:
+            ctx = await self._bot.get_context(interaction)
             await self.post_animal_picture(animal, ctx)
 
         return callback

tests/animals/test_cog.py

@@ -49,6 +49,14 @@ def create_fake_context(
     context.author.id = author_id
     context.channel.name = channel_name
     context.send = AsyncMock()
+    context.defer = AsyncMock()
+    # Provide a synchronous guild.channels list so discord.utils.get works.
+    guild = MagicMock()
+    fake_channel = MagicMock()
+    fake_channel.name = DEFAULT_CHANNEL_NAME
+    fake_channel.mention = f"#{DEFAULT_CHANNEL_NAME}"
+    guild.channels = [fake_channel]
+    context.guild = guild
     return context
 
 
@@ -58,19 +66,23 @@ async def test_command_success() -> None:
 
     await cog.post_animal_picture(DEFAULT_ANIMAL, ctx=ctx)
 
+    ctx.defer.assert_awaited_once()
     ctx.send.assert_awaited_once()
     embed = ctx.send.call_args.kwargs["embed"]
     assert embed.image.url == DEFAULT_IMAGE_URL
     assert embed.description == f"Behold! A friendly cat appeared from {DEFAULT_IMAGE_SOURCE}"
 
 
 async def test_command_wrong_channel() -> None:
-    cog = create_fake_cog(channel_name="channel-1")
-    ctx = create_fake_context(channel_name="channel-2")
+    cog = create_fake_cog(channel_name=DEFAULT_CHANNEL_NAME)
+    ctx = create_fake_context(channel_name="some-other-channel")
 
     await cog.post_animal_picture(DEFAULT_ANIMAL, ctx=ctx)
 
-    ctx.send.assert_not_called()
+    ctx.defer.assert_awaited_once()
+    ctx.send.assert_awaited_once()
+    (message,) = ctx.send.call_args.args
+    assert message == f"This command can only be used in #{DEFAULT_CHANNEL_NAME}."
 
 
 async def test_rate_limiting() -> None:
@@ -80,11 +92,14 @@ async def test_rate_limiting() -> None:
     ctx_1 = create_fake_context()
     await cog.post_animal_picture(DEFAULT_ANIMAL, ctx=ctx_1)
     ctx_1.send.assert_awaited_once()
+    assert "embed" in ctx_1.send.call_args.kwargs
 
-    # second call with same user
+    # second call with same user - rate limited, ephemeral error message
     ctx_2 = create_fake_context()
     await cog.post_animal_picture(DEFAULT_ANIMAL, ctx=ctx_2)
-    ctx_2.send.assert_not_awaited()
+    ctx_2.send.assert_awaited_once()
+    (message,) = ctx_2.send.call_args.args
+    assert message == "You are being rate limited. Please try again later."
 
 
 async def test_rate_limiting_different_user() -> None:
@@ -145,4 +160,7 @@ async def generate_image(self) -> AnimalImage | None:
 
     ctx.send.assert_called_once()
     (message,) = ctx.send.call_args.args
-    assert message == f"Failed to fetch {DEFAULT_ANIMAL} picture."
+    assert message == (
+        f"Failed to fetch {DEFAULT_ANIMAL} picture. If this happens repeatedly, please report it."
+    )
+    assert ctx.send.call_args.kwargs.get("ephemeral") is True