diff --git a/bot.py b/bot.py index 3e2f862..3500209 100644 --- a/bot.py +++ b/bot.py @@ -64,6 +64,44 @@ def setup_logging(): return logger +class MaintenanceAwareTree(discord.app_commands.CommandTree): + """ + CommandTree subclass that gates all interactions behind a maintenance mode check. + + When bot.maintenance_mode is True, non-administrator users receive an ephemeral + error message and the interaction is blocked. Administrators are always allowed + through. When maintenance_mode is False the check is a no-op and every + interaction proceeds normally. + + This is the correct way to register a global interaction_check for app commands + in discord.py — overriding the method on a CommandTree subclass passed via + tree_cls rather than attempting to assign a decorator to self.tree inside + setup_hook. + """ + + async def interaction_check(self, interaction: discord.Interaction) -> bool: + """Allow admins through; block everyone else when maintenance mode is active.""" + bot = interaction.client # type: ignore[assignment] + + # If maintenance mode is off, always allow. + if not getattr(bot, "maintenance_mode", False): + return True + + # Maintenance mode is on — let administrators through unconditionally. + if ( + isinstance(interaction.user, discord.Member) + and interaction.user.guild_permissions.administrator + ): + return True + + # Block non-admin users with an ephemeral notice. + await interaction.response.send_message( + "The bot is currently in maintenance mode. Please try again later.", + ephemeral=True, + ) + return False + + class SBABot(commands.Bot): """Custom bot class for SBA league management.""" @@ -77,8 +115,10 @@ class SBABot(commands.Bot): command_prefix="!", # Legacy prefix, primarily using slash commands intents=intents, description="Major Domo v2.0", + tree_cls=MaintenanceAwareTree, ) + self.maintenance_mode: bool = False self.logger = logging.getLogger("discord_bot_v2") self.maintenance_mode: bool = False diff --git a/commands/admin/management.py b/commands/admin/management.py index 8950c4c..b738e2c 100644 --- a/commands/admin/management.py +++ b/commands/admin/management.py @@ -490,7 +490,7 @@ class AdminCommands(commands.Cog): await interaction.response.defer() is_enabling = mode.lower() == "on" - self.bot.maintenance_mode = is_enabling + self.bot.maintenance_mode = is_enabling # type: ignore[attr-defined] self.logger.info( f"Maintenance mode {'enabled' if is_enabling else 'disabled'} by {interaction.user} (id={interaction.user.id})" ) diff --git a/tests/test_bot_maintenance_tree.py b/tests/test_bot_maintenance_tree.py new file mode 100644 index 0000000..c6530f9 --- /dev/null +++ b/tests/test_bot_maintenance_tree.py @@ -0,0 +1,282 @@ +""" +Tests for MaintenanceAwareTree and the maintenance_mode attribute on SBABot. + +What: + Verifies that the CommandTree subclass correctly gates interactions behind + bot.maintenance_mode. When maintenance mode is off every interaction is + allowed through unconditionally. When maintenance mode is on, non-admin + users receive an ephemeral error message and the check returns False, while + administrators are always allowed through. + +Why: + The original code attempted to register an interaction_check via a decorator + on self.tree inside setup_hook. That is not a valid pattern in discord.py — + interaction_check is an overridable async method on CommandTree, not a + decorator. The broken assignment caused a RuntimeWarning (unawaited + coroutine) and silently made maintenance mode a no-op. These tests confirm + the correct subclass-based implementation behaves as specified. +""" + +import pytest +from unittest.mock import AsyncMock, MagicMock + +import discord + +# --------------------------------------------------------------------------- +# Helpers / fixtures +# --------------------------------------------------------------------------- + + +def _make_bot(maintenance_mode: bool = False) -> MagicMock: + """Return a minimal mock bot with a maintenance_mode attribute.""" + bot = MagicMock() + bot.maintenance_mode = maintenance_mode + return bot + + +def _make_interaction(is_admin: bool, bot: MagicMock) -> AsyncMock: + """ + Build a mock discord.Interaction. + + The interaction's .client is set to *bot* so that MaintenanceAwareTree + can read bot.maintenance_mode via interaction.client, mirroring how + discord.py wires things at runtime. + """ + interaction = AsyncMock(spec=discord.Interaction) + interaction.client = bot + + # Mock the user as a guild Member so that guild_permissions is accessible. + user = MagicMock(spec=discord.Member) + user.guild_permissions = MagicMock() + user.guild_permissions.administrator = is_admin + interaction.user = user + + # response.send_message must be awaitable. + interaction.response = AsyncMock() + interaction.response.send_message = AsyncMock() + + return interaction + + +# --------------------------------------------------------------------------- +# Import the class under test after mocks are available. +# We import here (not at module level) so that the conftest env-vars are set +# before any discord_bot_v2 modules are touched. +# --------------------------------------------------------------------------- + + +@pytest.fixture(autouse=True) +def _patch_discord_app_commands(monkeypatch): + """ + Prevent MaintenanceAwareTree.__init__ from calling discord internals that + need a real event loop / Discord connection. We test only the logic of + interaction_check, so we stub out the parent __init__. + """ + # Nothing extra to patch for the interaction_check itself; the parent + # CommandTree.__init__ is only called when constructing SBABot, which we + # don't do in these unit tests. + yield + + +# --------------------------------------------------------------------------- +# Tests for MaintenanceAwareTree.interaction_check +# --------------------------------------------------------------------------- + + +class TestMaintenanceAwareTree: + """Unit tests for MaintenanceAwareTree.interaction_check.""" + + @pytest.fixture + def tree_cls(self): + """Import and return the MaintenanceAwareTree class.""" + from bot import MaintenanceAwareTree + + return MaintenanceAwareTree + + # ------------------------------------------------------------------ + # Maintenance OFF + # ------------------------------------------------------------------ + + @pytest.mark.asyncio + async def test_maintenance_off_allows_non_admin(self, tree_cls): + """ + When maintenance_mode is False, non-admin users are always allowed. + The check must return True without sending any message. + """ + bot = _make_bot(maintenance_mode=False) + interaction = _make_interaction(is_admin=False, bot=bot) + + # Instantiate tree without calling parent __init__ by testing the method + # directly on an unbound basis. + result = await tree_cls.interaction_check( + MagicMock(), # placeholder 'self' for the tree instance + interaction, + ) + + assert result is True + interaction.response.send_message.assert_not_called() + + @pytest.mark.asyncio + async def test_maintenance_off_allows_admin(self, tree_cls): + """ + When maintenance_mode is False, admin users are also always allowed. + """ + bot = _make_bot(maintenance_mode=False) + interaction = _make_interaction(is_admin=True, bot=bot) + + result = await tree_cls.interaction_check(MagicMock(), interaction) + + assert result is True + interaction.response.send_message.assert_not_called() + + # ------------------------------------------------------------------ + # Maintenance ON — non-admin + # ------------------------------------------------------------------ + + @pytest.mark.asyncio + async def test_maintenance_on_blocks_non_admin(self, tree_cls): + """ + When maintenance_mode is True, non-admin users must be blocked. + The check must return False and send an ephemeral message. + """ + bot = _make_bot(maintenance_mode=True) + interaction = _make_interaction(is_admin=False, bot=bot) + + result = await tree_cls.interaction_check(MagicMock(), interaction) + + assert result is False + interaction.response.send_message.assert_called_once() + + # Confirm the call used ephemeral=True + _, kwargs = interaction.response.send_message.call_args + assert kwargs.get("ephemeral") is True + + @pytest.mark.asyncio + async def test_maintenance_on_message_has_no_emoji(self, tree_cls): + """ + The maintenance block message must not contain emoji characters. + The project style deliberately strips emoji from user-facing strings. + """ + import unicodedata + + bot = _make_bot(maintenance_mode=True) + interaction = _make_interaction(is_admin=False, bot=bot) + + await tree_cls.interaction_check(MagicMock(), interaction) + + args, _ = interaction.response.send_message.call_args + message_text = args[0] if args else "" + + for ch in message_text: + category = unicodedata.category(ch) + assert category != "So", ( + f"Unexpected emoji/symbol character {ch!r} (category {category!r}) " + f"found in maintenance message: {message_text!r}" + ) + + # ------------------------------------------------------------------ + # Maintenance ON — admin + # ------------------------------------------------------------------ + + @pytest.mark.asyncio + async def test_maintenance_on_allows_admin(self, tree_cls): + """ + When maintenance_mode is True, administrator users must still be + allowed through. Admins should never be locked out of bot commands. + """ + bot = _make_bot(maintenance_mode=True) + interaction = _make_interaction(is_admin=True, bot=bot) + + result = await tree_cls.interaction_check(MagicMock(), interaction) + + assert result is True + interaction.response.send_message.assert_not_called() + + # ------------------------------------------------------------------ + # Edge case: non-Member user during maintenance + # ------------------------------------------------------------------ + + @pytest.mark.asyncio + async def test_maintenance_on_blocks_non_member_user(self, tree_cls): + """ + When maintenance_mode is True and the user is not a guild Member + (e.g. interaction from a DM context), the check must still block them + because we cannot verify administrator status. + """ + bot = _make_bot(maintenance_mode=True) + interaction = AsyncMock(spec=discord.Interaction) + interaction.client = bot + + # Simulate a non-Member user (e.g. discord.User from DM context) + user = MagicMock(spec=discord.User) + # discord.User has no guild_permissions attribute + interaction.user = user + interaction.response = AsyncMock() + interaction.response.send_message = AsyncMock() + + result = await tree_cls.interaction_check(MagicMock(), interaction) + + assert result is False + interaction.response.send_message.assert_called_once() + + # ------------------------------------------------------------------ + # Missing attribute safety: bot without maintenance_mode attr + # ------------------------------------------------------------------ + + @pytest.mark.asyncio + async def test_missing_maintenance_mode_attr_defaults_to_allowed(self, tree_cls): + """ + If the bot object does not have a maintenance_mode attribute (e.g. + during testing or unusual startup), getattr fallback must treat it as + False and allow the interaction. + """ + bot = MagicMock() + # Deliberately do NOT set bot.maintenance_mode + del bot.maintenance_mode + + interaction = _make_interaction(is_admin=False, bot=bot) + + result = await tree_cls.interaction_check(MagicMock(), interaction) + + assert result is True + + +# --------------------------------------------------------------------------- +# Tests for SBABot.maintenance_mode attribute +# --------------------------------------------------------------------------- + + +class TestSBABotMaintenanceModeAttribute: + """ + Confirms that SBABot.__init__ always sets maintenance_mode = False. + + We avoid constructing a real SBABot (which requires a Discord event loop + and valid token infrastructure) by patching the entire commands.Bot.__init__ + and then calling SBABot.__init__ directly on a bare instance so that only + the SBABot-specific attribute assignments execute. + """ + + def test_maintenance_mode_default_is_false(self, monkeypatch): + """ + SBABot.__init__ must set self.maintenance_mode = False so that the + MaintenanceAwareTree has the attribute available from the very first + interaction, even before /admin-maintenance is ever called. + + Strategy: patch commands.Bot.__init__ to be a no-op so super().__init__ + succeeds without a real Discord connection, then call SBABot.__init__ + and assert the attribute is present with the correct default value. + """ + from unittest.mock import patch + from discord.ext import commands + from bot import SBABot + + with patch.object(commands.Bot, "__init__", return_value=None): + bot = SBABot.__new__(SBABot) + SBABot.__init__(bot) + + assert hasattr( + bot, "maintenance_mode" + ), "SBABot must define self.maintenance_mode in __init__" + assert ( + bot.maintenance_mode is False + ), "SBABot.maintenance_mode must default to False"