Merge pull request 'feat: add is_admin() helper to utils/permissions.py (#55)' (#64) from ai/major-domo-v2#55 into next-release

Reviewed-on: #64

commands/profile/images.py

@@ -22,6 +22,7 @@ from utils.decorators import logged_command
 from views.embeds import EmbedColors, EmbedTemplate
 from views.base import BaseView
 from models.player import Player
+from utils.permissions import is_admin
 
 # URL Validation Functions
 
@@ -117,7 +118,7 @@ async def can_edit_player_image(
         If has permission, error_message is empty string
     """
     # Admins can edit anyone
-    if interaction.user.guild_permissions.administrator:
+    if is_admin(interaction):
         logger.debug("User is admin, granting permission", user_id=interaction.user.id)
         return True, ""
 

tests/test_commands_profile_images.py

@@ -3,17 +3,19 @@ Tests for player image management commands.
 
 Covers URL validation, permission checking, and command execution.
 """
+
 import pytest
 import asyncio
 from unittest.mock import MagicMock, patch
 import aiohttp
+import discord
 from aioresponses import aioresponses
 
 from commands.profile.images import (
     validate_url_format,
     check_url_accessibility,
     can_edit_player_image,
-    ImageCommands
+    ImageCommands,
 )
 from tests.factories import PlayerFactory, TeamFactory
 
@@ -94,7 +96,7 @@ class TestURLAccessibility:
         url = "https://example.com/image.jpg"
 
         with aioresponses() as m:
-            m.head(url, status=200, headers={'content-type': 'image/jpeg'})
+            m.head(url, status=200, headers={"content-type": "image/jpeg"})
 
             is_accessible, error = await check_url_accessibility(url)
 
@@ -118,7 +120,7 @@ class TestURLAccessibility:
         url = "https://example.com/page.html"
 
         with aioresponses() as m:
-            m.head(url, status=200, headers={'content-type': 'text/html'})
+            m.head(url, status=200, headers={"content-type": "text/html"})
 
             is_accessible, error = await check_url_accessibility(url)
 
@@ -157,6 +159,7 @@ class TestPermissionChecking:
     async def test_admin_can_edit_any_player(self):
         """Test administrator can edit any player's images."""
         mock_interaction = MagicMock()
+        mock_interaction.user = MagicMock(spec=discord.Member)
         mock_interaction.user.id = 12345
         mock_interaction.user.guild_permissions.administrator = True
 
@@ -186,7 +189,9 @@ class TestPermissionChecking:
 
         mock_logger = MagicMock()
 
-        with patch('commands.profile.images.team_service.get_teams_by_owner') as mock_get_teams:
+        with patch(
+            "commands.profile.images.team_service.get_teams_by_owner"
+        ) as mock_get_teams:
             mock_get_teams.return_value = [user_team]
 
             has_permission, error = await can_edit_player_image(
@@ -211,7 +216,9 @@ class TestPermissionChecking:
 
         mock_logger = MagicMock()
 
-        with patch('commands.profile.images.team_service.get_teams_by_owner') as mock_get_teams:
+        with patch(
+            "commands.profile.images.team_service.get_teams_by_owner"
+        ) as mock_get_teams:
             mock_get_teams.return_value = [user_team]
 
             has_permission, error = await can_edit_player_image(
@@ -236,7 +243,9 @@ class TestPermissionChecking:
 
         mock_logger = MagicMock()
 
-        with patch('commands.profile.images.team_service.get_teams_by_owner') as mock_get_teams:
+        with patch(
+            "commands.profile.images.team_service.get_teams_by_owner"
+        ) as mock_get_teams:
             mock_get_teams.return_value = [user_team]
 
             has_permission, error = await can_edit_player_image(
@@ -258,7 +267,9 @@ class TestPermissionChecking:
 
         mock_logger = MagicMock()
 
-        with patch('commands.profile.images.team_service.get_teams_by_owner') as mock_get_teams:
+        with patch(
+            "commands.profile.images.team_service.get_teams_by_owner"
+        ) as mock_get_teams:
             mock_get_teams.return_value = []
 
             has_permission, error = await can_edit_player_image(
@@ -299,7 +310,7 @@ class TestImageCommandsIntegration:
 
     async def test_set_image_command_structure(self, commands_cog):
         """Test that set_image command is properly configured."""
-        assert hasattr(commands_cog, 'set_image')
+        assert hasattr(commands_cog, "set_image")
         assert commands_cog.set_image.name == "set-image"
 
     async def test_fancy_card_updates_vanity_card_field(self, commands_cog):

utils/permissions.py

@@ -7,6 +7,7 @@ servers and user types:
 - @league_only: Only available in the league server
 - @requires_team: User must have a team (works with global commands)
 """
+
 import logging
 from functools import wraps
 from typing import Optional, Callable
@@ -21,6 +22,7 @@ logger = logging.getLogger(__name__)
 
 class PermissionError(Exception):
     """Raised when a user doesn't have permission to use a command."""
+
     pass
 
 
@@ -54,17 +56,16 @@ async def get_user_team(user_id: int) -> Optional[dict]:
     # This call is automatically cached by TeamService
     config = get_config()
     team = await team_service.get_team_by_owner(
-        owner_id=user_id,
+        owner_id=user_id, season=config.sba_season
-        season=config.sba_season
     )
 
     if team:
         logger.debug(f"User {user_id} has team: {team.lname}")
         return {
-            'id': team.id,
+            "id": team.id,
-            'name': team.lname,
+            "name": team.lname,
-            'abbrev': team.abbrev,
+            "abbrev": team.abbrev,
-            'season': team.season
+            "season": team.season,
         }
 
     logger.debug(f"User {user_id} does not have a team")
@@ -77,6 +78,18 @@ def is_league_server(guild_id: int) -> bool:
     return guild_id == config.guild_id
 
 
+def is_admin(interaction: discord.Interaction) -> bool:
+    """Check if the interaction user is a server administrator.
+
+    Includes an isinstance guard for discord.Member so this is safe
+    to call from DM contexts (where guild_permissions is unavailable).
+    """
+    return (
+        isinstance(interaction.user, discord.Member)
+        and interaction.user.guild_permissions.administrator
+    )
+
+
 def league_only():
     """
     Decorator to restrict a command to the league server only.
@@ -87,14 +100,14 @@ def league_only():
         async def team_command(self, interaction: discord.Interaction):
             # Only executes in league server
     """
+
     def decorator(func: Callable) -> Callable:
         @wraps(func)
         async def wrapper(self, interaction: discord.Interaction, *args, **kwargs):
             # Check if in a guild
             if not interaction.guild:
                 await interaction.response.send_message(
-                    "❌ This command can only be used in a server.",
+                    "❌ This command can only be used in a server.", ephemeral=True
-                    ephemeral=True
                 )
                 return
 
@@ -102,13 +115,14 @@ def league_only():
             if not is_league_server(interaction.guild.id):
                 await interaction.response.send_message(
                     "❌ This command is only available in the SBa league server.",
-                    ephemeral=True
+                    ephemeral=True,
                 )
                 return
 
             return await func(self, interaction, *args, **kwargs)
 
         return wrapper
+
     return decorator
 
 
@@ -123,6 +137,7 @@ def requires_team():
         async def mymoves_command(self, interaction: discord.Interaction):
             # Only executes if user has a team
     """
+
     def decorator(func: Callable) -> Callable:
         @wraps(func)
         async def wrapper(self, interaction: discord.Interaction, *args, **kwargs):
@@ -133,29 +148,33 @@ def requires_team():
                 if team is None:
                     await interaction.response.send_message(
                         "❌ This command requires you to have a team in the SBa league. Contact an admin if you believe this is an error.",
-                        ephemeral=True
+                        ephemeral=True,
                     )
                     return
 
                 # Store team info in interaction for command to use
                 # This allows commands to access the team without another lookup
-                interaction.extras['user_team'] = team
+                interaction.extras["user_team"] = team
 
                 return await func(self, interaction, *args, **kwargs)
 
             except Exception as e:
                 # Log the error for debugging
-                logger.error(f"Error checking team ownership for user {interaction.user.id}: {e}", exc_info=True)
+                logger.error(
+                    f"Error checking team ownership for user {interaction.user.id}: {e}",
+                    exc_info=True,
+                )
 
                 # Provide helpful error message to user
                 await interaction.response.send_message(
                     "❌ Unable to verify team ownership due to a temporary error. Please try again in a moment. "
                     "If this persists, contact an admin.",
-                    ephemeral=True
+                    ephemeral=True,
                 )
                 return
 
         return wrapper
+
     return decorator
 
 
@@ -170,12 +189,14 @@ def global_command():
         async def roll_command(self, interaction: discord.Interaction):
             # Available in all servers
     """
+
     def decorator(func: Callable) -> Callable:
         @wraps(func)
         async def wrapper(self, interaction: discord.Interaction, *args, **kwargs):
             return await func(self, interaction, *args, **kwargs)
 
         return wrapper
+
     return decorator
 
 
@@ -190,35 +211,35 @@ def admin_only():
         async def sync_command(self, interaction: discord.Interaction):
             # Only executes for admins
     """
+
     def decorator(func: Callable) -> Callable:
         @wraps(func)
         async def wrapper(self, interaction: discord.Interaction, *args, **kwargs):
             # Check if user is guild admin
             if not interaction.guild:
                 await interaction.response.send_message(
-                    "❌ This command can only be used in a server.",
+                    "❌ This command can only be used in a server.", ephemeral=True
-                    ephemeral=True
                 )
                 return
 
             # Check if user has admin permissions
             if not isinstance(interaction.user, discord.Member):
                 await interaction.response.send_message(
-                    "❌ Unable to verify permissions.",
+                    "❌ Unable to verify permissions.", ephemeral=True
-                    ephemeral=True
                 )
                 return
 
             if not interaction.user.guild_permissions.administrator:
                 await interaction.response.send_message(
                     "❌ This command requires administrator permissions.",
-                    ephemeral=True
+                    ephemeral=True,
                 )
                 return
 
             return await func(self, interaction, *args, **kwargs)
 
         return wrapper
+
     return decorator
 
 
@@ -241,6 +262,7 @@ def league_admin_only():
         async def admin_sync_prefix(self, ctx: commands.Context):
             # Only league server admins can use this
     """
+
     def decorator(func: Callable) -> Callable:
         @wraps(func)
         async def wrapper(self, ctx_or_interaction, *args, **kwargs):
@@ -254,6 +276,7 @@ def league_admin_only():
 
                 async def send_error(msg: str):
                     await ctx.send(msg)
+
             else:
                 interaction = ctx_or_interaction
                 guild = interaction.guild
@@ -269,7 +292,9 @@ def league_admin_only():
 
             # Check if league server
             if not is_league_server(guild.id):
-                await send_error("❌ This command is only available in the SBa league server.")
+                await send_error(
+                    "❌ This command is only available in the SBa league server."
+                )
                 return
 
             # Check admin permissions
@@ -284,4 +309,5 @@ def league_admin_only():
             return await func(self, ctx_or_interaction, *args, **kwargs)
 
         return wrapper
+
     return decorator