Add /branding command for team color/logo management (v2.20.0)

Implemented comprehensive team branding management system allowing team owners
to update colors and logos for major league, minor league, and dice rolls.

Features:
- Modal-based interactive form input with validation
- Hex color validation with normalization (6 chars, optional # prefix)
- Image URL accessibility testing with aiohttp (5 second timeout)
- Preview + confirmation workflow with ConfirmationView
- Support for both major league and minor league affiliate updates
- Dice color customization for game rolls
- Discord role color sync (non-blocking with graceful fallback)
- Comprehensive error handling and user feedback

Technical Implementation:
- BrandingModal class with 5 optional fields
- Concurrent URL validation using asyncio.gather
- Fixed team_service.update_team() to use PATCH with query parameters
- Enhanced TeamService documentation with correct method signatures
- 33 comprehensive tests (100% passing)

Bug Fixes:
- Fixed modal send timing (immediate response vs deferred)
- Fixed interaction handling for cancel button
- Fixed database API communication (PATCH query params vs PUT JSON)

Files:
- commands/teams/branding.py (NEW - ~500 lines)
- commands/teams/__init__.py (added BrandingCommands registration)
- commands/teams/CLAUDE.md (added comprehensive documentation)
- tests/test_commands_teams_branding.py (NEW - 33 tests)
- services/team_service.py (fixed update_team to use query params)
- VERSION (2.19.2 → 2.20.0)

Docker: manticorum67/major-domo-discord-app-v2:2.20.0

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

VERSION

@@ -1 +1 @@
-2.19.2
+2.20.0

commands/teams/CLAUDE.md

@@ -26,6 +26,15 @@ This directory contains Discord slash commands for team information and roster m
   - `team_service.get_team_by_abbrev()`
   - `team_service.get_team_roster()`
 
+### `branding.py`
+- **Command**: `/branding`
+- **Description**: Update team colors and logos via interactive modal
+- **Access**: Team owners only (verified via `team_service.get_team_by_owner()`)
+- **Service Dependencies**:
+  - `team_service.get_team_by_owner()`
+  - `team_service.update_team()`
+  - `models.team.Team.minor_league_affiliate()`
+
 ## Key Features
 
 ### Team Information Display (`info.py`)
@@ -67,6 +76,49 @@ This directory contains Discord slash commands for team information and roster m
 - **Player Organization**: Batters and pitchers grouped separately
 - **Chunked Display**: Long player lists split across multiple fields
 
+### Team Branding Management (`branding.py`)
+- **Modal-Based Input**: Interactive form with 5 optional fields
+- **Preview + Confirmation**: Visual preview before applying changes
+- **Multi-Team Support**: Updates major league and minor league teams
+- **Discord Integration**: Attempts to update Discord role colors (non-blocking)
+
+#### Workflow
+1. User runs `/branding`
+2. Bot verifies user owns a team
+3. Modal displays with 5 optional fields showing current values
+4. User fills in desired changes, submits
+5. Bot validates all inputs (hex colors, URL accessibility)
+6. Bot shows preview embeds with confirmation buttons
+7. User confirms or cancels
+8. Bot applies changes to database
+9. Bot attempts to update Discord role color (non-blocking)
+10. Bot shows success message with details
+
+#### Modal Fields
+- **Major League Team Color** - 6-character hex code (e.g., FF5733 or #FF5733)
+- **Major League Logo URL** - Public image URL (.png, .jpg, .jpeg, .gif, .webp)
+- **Minor League Team Color** - 6-character hex code
+- **Minor League Logo URL** - Public image URL
+- **Dice Roll Color** - 6-character hex code for dice displays
+
+#### Validation Rules
+- **Hex Colors**: Must be exactly 6 characters, valid hex digits (0-9, A-F), # prefix optional
+- **Image URLs**: Must start with http:// or https://, end with valid extension, be publicly accessible
+- **All Fields Optional**: Leave blank to keep current value
+- **URL Accessibility**: Tests URLs with HEAD request (5 second timeout)
+- **Content Type Check**: Verifies URLs point to images
+
+#### Database Updates
+- Major league team: `color`, `thumbnail`, `dice_color` fields
+- Minor league team: `color`, `thumbnail` fields
+- Uses `team_service.update_team()` for all database operations
+
+#### Discord Role Updates
+- Bot attempts to update Discord role color to match team color
+- Failures are non-blocking (show warning but database updates succeed)
+- Common failure reasons: role not found, missing permissions
+- Updates role by matching `team.lname` (long name)
+
 ## Architecture Notes
 
 ### Embed Design
@@ -109,6 +161,13 @@ This directory contains Discord slash commands for team information and roster m
    - Verify standings data structure matches expected format
    - Ensure error handling for malformed standings data
 
+5. **Branding command issues**:
+   - **"You don't own a team"**: User is not registered as team owner for current season
+   - **"URL not accessible"**: Image URL returned non-200 status or timed out (check URL is public)
+   - **"Color must be 6 characters"**: Hex color is wrong length or contains invalid characters
+   - **"Discord role update failed"**: Role color couldn't be updated (database still succeeded - not critical)
+   - **"No minor league affiliate"**: Team doesn't have MiL team (this is normal for some teams)
+
 ### Dependencies
 - `services.team_service`
 - `models.team.Team`
@@ -118,7 +177,9 @@ This directory contains Discord slash commands for team information and roster m
 - `exceptions.BotException`
 
 ### Testing
-Run tests with: `python -m pytest tests/test_commands_teams.py -v`
+Run tests with:
+- All team commands: `python -m pytest tests/test_commands_teams.py -v`
+- Branding command only: `python -m pytest tests/test_commands_teams_branding.py -v`
 
 ## Database Requirements
 - Team records with abbreviations, names, colors, logos

commands/teams/__init__.py

@@ -11,6 +11,7 @@ from discord.ext import commands
 
 from .info import TeamInfoCommands
 from .roster import TeamRosterCommands
+from .branding import BrandingCommands
 
 logger = logging.getLogger(f'{__name__}.setup_teams')
 
@@ -25,6 +26,7 @@ async def setup_teams(bot: commands.Bot) -> Tuple[int, int, List[str]]:
     team_cogs: List[Tuple[str, Type[commands.Cog]]] = [
         ("TeamInfoCommands", TeamInfoCommands),
         ("TeamRosterCommands", TeamRosterCommands),
+        ("BrandingCommands", BrandingCommands),
     ]
     
     successful = 0

commands/teams/branding.py

@@ -0,0 +1,677 @@
+"""
+Team Branding Management Commands
+
+Allows team owners to update their team's visual branding including colors and logos
+for major league teams, minor league affiliates, and dice roll displays.
+"""
+import asyncio
+import aiohttp
+from typing import Optional, Tuple, Dict, List
+
+import discord
+from discord.ext import commands
+from discord import app_commands
+
+from config import get_config
+from services import team_service
+from models.team import Team
+from utils.logging import get_contextual_logger
+from utils.decorators import logged_command
+from views.embeds import EmbedTemplate, EmbedColors
+from views.confirmations import ConfirmationView
+
+
+# ============================================================================
+# Validation Functions
+# ============================================================================
+
+def validate_hex_color(color: str) -> Tuple[bool, str, str]:
+    """
+    Validate hex color format.
+
+    Args:
+        color: Hex color string (with or without # prefix)
+
+    Returns:
+        Tuple of (is_valid, normalized_color, error_message)
+        - is_valid: True if validation passed
+        - normalized_color: Uppercase hex without # prefix
+        - error_message: Empty if valid, error description if invalid
+    """
+    if not color:
+        return True, "", ""
+
+    # Strip # and whitespace
+    color = color.strip().lstrip('#')
+
+    # Check length
+    if len(color) != 6:
+        return False, "", "Color must be 6 characters (e.g., FF5733 or #FF5733)"
+
+    # Check characters are valid hex
+    if not all(c in '0123456789ABCDEFabcdef' for c in color):
+        return False, "", "Color must contain only hex digits (0-9, A-F)"
+
+    return True, color.upper(), ""
+
+
+async def validate_image_url(url: str) -> Tuple[bool, str]:
+    """
+    Validate image URL format and accessibility.
+
+    Args:
+        url: Image URL to validate
+
+    Returns:
+        Tuple of (is_valid, error_message)
+        - is_valid: True if validation passed
+        - error_message: Empty if valid, error description if invalid
+    """
+    if not url:
+        return True, ""
+
+    # Format validation
+    if not url.startswith(('http://', 'https://')):
+        return False, "URL must start with http:// or https://"
+
+    valid_extensions = ('.png', '.jpg', '.jpeg', '.gif', '.webp')
+    # Check extension (handle query parameters)
+    url_path = url.split('?')[0]  # Strip query params
+    if not any(url_path.lower().endswith(ext) for ext in valid_extensions):
+        return False, f"URL must end with {', '.join(valid_extensions)}"
+
+    # Accessibility test
+    try:
+        async with aiohttp.ClientSession() as session:
+            async with session.head(url, timeout=aiohttp.ClientTimeout(total=5)) as resp:
+                if resp.status != 200:
+                    return False, f"URL returned status {resp.status} (not accessible)"
+
+                content_type = resp.headers.get('Content-Type', '')
+                if not content_type.startswith('image/'):
+                    return False, "URL does not point to an image"
+
+        return True, ""
+    except asyncio.TimeoutError:
+        return False, "URL request timed out (5 seconds)"
+    except Exception as e:
+        return False, f"Unable to access URL: {str(e)}"
+
+
+# ============================================================================
+# Branding Modal
+# ============================================================================
+
+class BrandingModal(discord.ui.Modal, title="Team Branding"):
+    """Modal form for collecting team branding updates."""
+
+    def __init__(
+        self,
+        cog,  # BrandingCommands instance
+        ml_team: Team,
+        mil_team: Optional[Team]
+    ):
+        """
+        Initialize branding modal with current team values.
+
+        Args:
+            cog: BrandingCommands cog instance (for access to service and logger)
+            ml_team: Major league team to update
+            mil_team: Minor league team (optional)
+        """
+        super().__init__()
+        self.cog = cog
+        self.ml_team = ml_team
+        self.mil_team = mil_team
+
+        # Set placeholders to show current values
+        current_ml_color = ml_team.color or "Not set"
+        current_dice_color = ml_team.dice_color or "Not set"
+        current_mil_color = mil_team.color if mil_team else "No MiL team"
+
+        self.major_color.placeholder = f"Current: {current_ml_color}"
+        self.dice_color.placeholder = f"Current: {current_dice_color}"
+        self.minor_color.placeholder = f"Current: {current_mil_color}"
+
+    major_color = discord.ui.TextInput(
+        label="Major League Team Color",
+        placeholder="FF5733 or #FF5733",
+        max_length=7,
+        required=False,
+        style=discord.TextStyle.short
+    )
+
+    major_logo = discord.ui.TextInput(
+        label="Major League Logo URL",
+        placeholder="https://... (leave blank to keep current)",
+        max_length=500,
+        required=False,
+        style=discord.TextStyle.short
+    )
+
+    minor_color = discord.ui.TextInput(
+        label="Minor League Team Color",
+        placeholder="33C3FF or #33C3FF",
+        max_length=7,
+        required=False,
+        style=discord.TextStyle.short
+    )
+
+    minor_logo = discord.ui.TextInput(
+        label="Minor League Logo URL",
+        placeholder="https://... (leave blank to keep current)",
+        max_length=500,
+        required=False,
+        style=discord.TextStyle.short
+    )
+
+    dice_color = discord.ui.TextInput(
+        label="Dice Roll Color",
+        placeholder="A6CE39 or #A6CE39",
+        max_length=7,
+        required=False,
+        style=discord.TextStyle.short
+    )
+
+    async def on_submit(self, interaction: discord.Interaction):
+        """Handle modal submission and validation."""
+        await interaction.response.defer()
+
+        # Collect modal data
+        modal_data = {
+            'major_color': self.major_color.value.strip(),
+            'major_logo': self.major_logo.value.strip(),
+            'minor_color': self.minor_color.value.strip(),
+            'minor_logo': self.minor_logo.value.strip(),
+            'dice_color': self.dice_color.value.strip(),
+        }
+
+        # Check if any fields were filled
+        if not any(modal_data.values()):
+            embed = EmbedTemplate.info(
+                "No Changes",
+                "No branding changes were specified. All fields were left blank."
+            )
+            await interaction.followup.send(embed=embed, ephemeral=True)
+            return
+
+        # Validate and process
+        await self.cog._process_branding_update(
+            interaction,
+            self.ml_team,
+            self.mil_team,
+            modal_data
+        )
+
+
+# ============================================================================
+# Branding Commands Cog
+# ============================================================================
+
+class BrandingCommands(commands.Cog):
+    """Team branding management command handlers."""
+
+    def __init__(self, bot: commands.Bot):
+        self.bot = bot
+        self.logger = get_contextual_logger(f'{__name__}.BrandingCommands')
+        self.logger.info("BrandingCommands cog initialized")
+
+    @app_commands.command(name="branding", description="Update your team's colors and logos")
+    @logged_command("/branding")
+    async def team_branding(self, interaction: discord.Interaction):
+        """
+        Update team branding including colors and logos.
+
+        Team owners can update:
+        - Major league team color and logo
+        - Minor league team color and logo
+        - Dice roll color
+        """
+        # Get current season
+        config = get_config()
+        season = config.sba_current_season
+
+        # Verify user owns a team (must do this BEFORE responding to interaction)
+        ml_team = await team_service.get_team_by_owner(interaction.user.id, season)
+
+        if not ml_team:
+            self.logger.info("User does not own a team", user_id=interaction.user.id)
+            embed = EmbedTemplate.error(
+                "Not a Team Owner",
+                "You don't own a team in the current season."
+            )
+            await interaction.response.send_message(embed=embed, ephemeral=True)
+            return
+
+        # Get minor league affiliate (if exists)
+        mil_team = None
+        try:
+            mil_team = await ml_team.minor_league_affiliate()
+            self.logger.info("Found minor league affiliate", mil_team_id=mil_team.id)
+        except ValueError:
+            # No MiL affiliate - this is OK
+            self.logger.info("No minor league affiliate found for team", team_id=ml_team.id)
+
+        # Show branding modal as immediate response
+        modal = BrandingModal(
+            cog=self,
+            ml_team=ml_team,
+            mil_team=mil_team
+        )
+
+        # Send modal as the interaction response
+        await interaction.response.send_modal(modal)
+
+    async def _process_branding_update(
+        self,
+        interaction: discord.Interaction,
+        ml_team: Team,
+        mil_team: Optional[Team],
+        modal_data: Dict[str, str]
+    ):
+        """
+        Process and validate branding update from modal.
+
+        Args:
+            interaction: Discord interaction
+            ml_team: Major league team to update
+            mil_team: Minor league team (optional)
+            modal_data: Dictionary of modal field values
+        """
+        # Validate all inputs
+        updates, errors = await self._validate_all_inputs(modal_data)
+
+        # Show validation errors
+        if errors:
+            error_text = "\n".join(errors)
+            embed = EmbedTemplate.error(
+                "Validation Errors",
+                f"{error_text}\n\n💡 **Tip:** Leave fields blank to keep current values"
+            )
+            await interaction.followup.send(embed=embed, ephemeral=True)
+            return
+
+        # Check if there are any actual updates
+        if not updates['major'] and not updates['minor']:
+            embed = EmbedTemplate.info(
+                "No Changes",
+                "No valid branding changes were specified."
+            )
+            await interaction.followup.send(embed=embed, ephemeral=True)
+            return
+
+        # Create preview embeds
+        embeds = await self._create_preview_embeds(ml_team, mil_team, updates)
+
+        # Show confirmation dialog
+        confirmation_view = ConfirmationView(
+            responders=[interaction.user],
+            timeout=60.0,
+            confirm_label="Apply Changes",
+            cancel_label="Cancel"
+        )
+
+        confirmation_message = await interaction.followup.send(
+            content="🎨 **Branding Preview** - Review and confirm changes:",
+            embeds=embeds,
+            view=confirmation_view,
+            wait=True
+        )
+
+        await confirmation_view.wait()
+
+        if not confirmation_view.confirmed:
+            embed = EmbedTemplate.info(
+                "Cancelled",
+                "Branding changes were not applied."
+            )
+            await confirmation_message.edit(content=None, embeds=[embed], view=None)
+            return
+
+        # Apply updates
+        await self._apply_branding_updates(interaction, ml_team, mil_team, updates, confirmation_message)
+
+    async def _validate_all_inputs(
+        self,
+        modal_data: Dict[str, str]
+    ) -> Tuple[Dict[str, Dict[str, str]], List[str]]:
+        """
+        Validate all modal inputs.
+
+        Args:
+            modal_data: Dictionary of modal field values
+
+        Returns:
+            Tuple of (updates_dict, error_list)
+            - updates_dict: {'major': {...}, 'minor': {...}}
+            - error_list: List of error messages
+        """
+        updates = {
+            'major': {},
+            'minor': {},
+        }
+        errors = []
+
+        # Validate major league color
+        if modal_data['major_color']:
+            valid, normalized, error = validate_hex_color(modal_data['major_color'])
+            if not valid:
+                errors.append(f"**Major Team Color:** {error}")
+            else:
+                updates['major']['color'] = normalized
+
+        # Validate dice color
+        if modal_data['dice_color']:
+            valid, normalized, error = validate_hex_color(modal_data['dice_color'])
+            if not valid:
+                errors.append(f"**Dice Roll Color:** {error}")
+            else:
+                updates['major']['dice_color'] = normalized
+
+        # Validate minor league color
+        if modal_data['minor_color']:
+            valid, normalized, error = validate_hex_color(modal_data['minor_color'])
+            if not valid:
+                errors.append(f"**Minor Team Color:** {error}")
+            else:
+                updates['minor']['color'] = normalized
+
+        # Collect URLs for concurrent validation
+        url_validations = []
+
+        if modal_data['major_logo']:
+            url_validations.append(('major_logo', 'Major Team Logo', modal_data['major_logo']))
+
+        if modal_data['minor_logo']:
+            url_validations.append(('minor_logo', 'Minor Team Logo', modal_data['minor_logo']))
+
+        # Validate all URLs concurrently
+        if url_validations:
+            tasks = [validate_image_url(url) for _, _, url in url_validations]
+            results = await asyncio.gather(*tasks)
+
+            for (field, label, url), (valid, error) in zip(url_validations, results):
+                if not valid:
+                    errors.append(f"**{label}:** {error}")
+                else:
+                    # Add to appropriate updates dict
+                    if field == 'major_logo':
+                        updates['major']['thumbnail'] = url
+                    elif field == 'minor_logo':
+                        updates['minor']['thumbnail'] = url
+
+        return updates, errors
+
+    async def _create_preview_embeds(
+        self,
+        ml_team: Team,
+        mil_team: Optional[Team],
+        updates: Dict[str, Dict[str, str]]
+    ) -> List[discord.Embed]:
+        """
+        Create preview embeds showing branding changes.
+
+        Args:
+            ml_team: Major league team
+            mil_team: Minor league team (optional)
+            updates: Updates dictionary from validation
+
+        Returns:
+            List of 1-3 embeds showing previews
+        """
+        embeds = []
+
+        # Major league preview
+        if updates['major']:
+            # Determine preview color (use new color if provided, else current)
+            if 'color' in updates['major']:
+                preview_color = int(updates['major']['color'], 16)
+            elif ml_team.color:
+                preview_color = int(ml_team.color, 16)
+            else:
+                preview_color = EmbedColors.PRIMARY
+
+            # Determine preview logo
+            preview_logo = updates['major'].get('thumbnail', ml_team.thumbnail)
+
+            embed = EmbedTemplate.create_base_embed(
+                title=f"{ml_team.lname} - Preview",
+                description="Major League Team Branding",
+                color=preview_color
+            )
+
+            if preview_logo:
+                embed.set_thumbnail(url=preview_logo)
+
+            # Add fields showing what's changing
+            changes = []
+            if 'color' in updates['major']:
+                changes.append(f"**Team Color:** #{updates['major']['color']}")
+            if 'thumbnail' in updates['major']:
+                changes.append(f"**Team Logo:** Updated ✓")
+            if 'dice_color' in updates['major']:
+                changes.append(f"**Dice Color:** #{updates['major']['dice_color']}")
+
+            if changes:
+                embed.add_field(
+                    name="Changes",
+                    value="\n".join(changes),
+                    inline=False
+                )
+
+            embeds.append(embed)
+
+        # Minor league preview (if applicable)
+        if mil_team and updates['minor']:
+            # Determine preview color
+            if 'color' in updates['minor']:
+                preview_color = int(updates['minor']['color'], 16)
+            elif mil_team.color:
+                preview_color = int(mil_team.color, 16)
+            else:
+                preview_color = EmbedColors.PRIMARY
+
+            # Determine preview logo
+            preview_logo = updates['minor'].get('thumbnail', mil_team.thumbnail)
+
+            embed = EmbedTemplate.create_base_embed(
+                title=f"{mil_team.lname} - Preview",
+                description="Minor League Team Branding",
+                color=preview_color
+            )
+
+            if preview_logo:
+                embed.set_thumbnail(url=preview_logo)
+
+            # Add fields showing what's changing
+            changes = []
+            if 'color' in updates['minor']:
+                changes.append(f"**Team Color:** #{updates['minor']['color']}")
+            if 'thumbnail' in updates['minor']:
+                changes.append(f"**Team Logo:** Updated ✓")
+
+            if changes:
+                embed.add_field(
+                    name="Changes",
+                    value="\n".join(changes),
+                    inline=False
+                )
+
+            embeds.append(embed)
+
+        # Dice color preview (if updated and different from team color)
+        if 'dice_color' in updates.get('major', {}):
+            dice_color_int = int(updates['major']['dice_color'], 16)
+
+            dice_embed = EmbedTemplate.create_base_embed(
+                title=f"{ml_team.lname} - Dice Color Preview",
+                description="This color will be used for dice rolls in gameplay",
+                color=dice_color_int
+            )
+
+            dice_embed.add_field(
+                name="Dice Color",
+                value=f"#{updates['major']['dice_color']}",
+                inline=False
+            )
+
+            embeds.append(dice_embed)
+
+        return embeds
+
+    async def _apply_branding_updates(
+        self,
+        interaction: discord.Interaction,
+        ml_team: Team,
+        mil_team: Optional[Team],
+        updates: Dict[str, Dict[str, str]],
+        confirmation_message
+    ):
+        """
+        Apply branding updates to database and Discord.
+
+        Args:
+            interaction: Discord interaction
+            ml_team: Major league team to update
+            mil_team: Minor league team (optional)
+            updates: Updates dictionary from validation
+            confirmation_message: The message to update with results
+        """
+        role_updated = False
+        role_error = None
+
+        # Update major league team
+        if updates['major']:
+            self.logger.info("Updating major league team", team_id=ml_team.id, updates=updates['major'])
+            updated_ml = await team_service.update_team(ml_team.id, updates['major'])
+
+            if not updated_ml:
+                self.logger.error("Failed to update major league team", team_id=ml_team.id)
+                embed = EmbedTemplate.error(
+                    "Update Failed",
+                    "Failed to update major league team branding. Please try again."
+                )
+                await confirmation_message.edit(content=None, embeds=[embed], view=None)
+                return
+
+            # Update Discord role color (non-blocking)
+            if 'color' in updates['major']:
+                role_updated, role_error = await self._update_discord_role_color(
+                    interaction,
+                    ml_team,
+                    updates['major']['color']
+                )
+
+        # Update minor league team (if applicable)
+        if mil_team and updates['minor']:
+            self.logger.info("Updating minor league team", team_id=mil_team.id, updates=updates['minor'])
+            await team_service.update_team(mil_team.id, updates['minor'])
+
+        # Create success message
+        success_message = self._format_success_message(updates, role_updated, role_error)
+
+        embed = EmbedTemplate.success(
+            "Branding Updated",
+            success_message
+        )
+
+        await confirmation_message.edit(content=None, embeds=[embed], view=None)
+
+    async def _update_discord_role_color(
+        self,
+        interaction: discord.Interaction,
+        team: Team,
+        hex_color: str
+    ) -> Tuple[bool, Optional[str]]:
+        """
+        Update Discord role color for team (non-blocking).
+
+        Args:
+            interaction: Discord interaction
+            team: Team whose role to update
+            hex_color: New hex color (without # prefix)
+
+        Returns:
+            Tuple of (success, error_message)
+        """
+        try:
+            # Find role by team long name
+            role = discord.utils.get(
+                interaction.guild.roles,
+                name=team.lname
+            )
+
+            if not role:
+                self.logger.warning("Discord role not found", team_name=team.lname)
+                return False, "Discord role not found"
+
+            # Convert hex to int
+            color_int = int(hex_color, 16)
+
+            # Update role
+            await role.edit(colour=color_int)
+
+            self.logger.info("Discord role color updated", team_name=team.lname, color=hex_color)
+            return True, None
+
+        except discord.Forbidden:
+            self.logger.warning("Missing permissions to edit role", team_name=team.lname)
+            return False, "Missing permissions to edit role"
+        except Exception as e:
+            self.logger.warning(f"Failed to update Discord role color: {e}", team_name=team.lname)
+            return False, str(e)
+
+    def _format_success_message(
+        self,
+        updates: Dict[str, Dict[str, str]],
+        role_updated: bool,
+        role_error: Optional[str]
+    ) -> str:
+        """
+        Format success message showing all applied changes.
+
+        Args:
+            updates: Updates dictionary
+            role_updated: Whether Discord role was updated
+            role_error: Error message if role update failed
+
+        Returns:
+            Formatted success message string
+        """
+        lines = []
+
+        # Major league updates
+        if updates['major']:
+            lines.append("**Major League:**")
+            if 'color' in updates['major']:
+                lines.append(f"• Color: #{updates['major']['color']} ✅")
+            if 'thumbnail' in updates['major']:
+                lines.append(f"• Logo: Updated ✅")
+            if 'dice_color' in updates['major']:
+                lines.append(f"• Dice Color: #{updates['major']['dice_color']} ✅")
+
+            # Discord role status
+            if 'color' in updates['major']:
+                if role_updated:
+                    lines.append(f"• Discord role: Updated ✅")
+                else:
+                    lines.append(f"• Discord role: Failed ({role_error}) ⚠️")
+
+        # Minor league updates
+        if updates['minor']:
+            lines.append("\n**Minor League:**")
+            if 'color' in updates['minor']:
+                lines.append(f"• Color: #{updates['minor']['color']} ✅")
+            if 'thumbnail' in updates['minor']:
+                lines.append(f"• Logo: Updated ✅")
+
+        # Warning about role update failure
+        if role_error:
+            lines.append("\n⚠️ **Note:** Discord role color update failed, but database was updated successfully.")
+
+        return "\n".join(lines)
+
+
+async def setup(bot: commands.Bot):
+    """Load the branding commands cog."""
+    await bot.add_cog(BrandingCommands(bot))

services/team_service.py

@@ -273,16 +273,17 @@ class TeamService(BaseService[Team]):
     async def update_team(self, team_id: int, updates: dict) -> Optional[Team]:
         """
         Update team information.
-        
+
         Args:
             team_id: Team ID to update
             updates: Dictionary of fields to update
-            
+
         Returns:
             Updated team instance or None
         """
         try:
-            return await self.update(team_id, updates)
+            # Use PATCH with query parameters (database API expects this)
+            return await self.patch(team_id, updates, use_query_params=True)
         except Exception as e:
             logger.error(f"Failed to update team {team_id}: {e}")
             return None

tests/test_commands_teams_branding.py

@@ -0,0 +1,482 @@
+"""
+Tests for team branding management commands.
+
+Covers validation functions, permission checking, and command execution.
+"""
+import pytest
+import asyncio
+from unittest.mock import AsyncMock, MagicMock, patch
+from aioresponses import aioresponses
+
+from commands.teams.branding import (
+    validate_hex_color,
+    validate_image_url,
+    BrandingCommands
+)
+from models.team import Team
+from tests.factories import TeamFactory
+
+
+class TestHexColorValidation:
+    """Test hex color format validation."""
+
+    def test_valid_hex_no_prefix(self):
+        """Test that valid hex without # prefix passes validation."""
+        is_valid, normalized, error = validate_hex_color("FF5733")
+        assert is_valid is True
+        assert normalized == "FF5733"
+        assert error == ""
+
+    def test_valid_hex_with_prefix(self):
+        """Test that valid hex with # prefix passes validation."""
+        is_valid, normalized, error = validate_hex_color("#FF5733")
+        assert is_valid is True
+        assert normalized == "FF5733"  # Prefix stripped
+        assert error == ""
+
+    def test_valid_hex_lowercase(self):
+        """Test that lowercase hex is normalized to uppercase."""
+        is_valid, normalized, error = validate_hex_color("ff5733")
+        assert is_valid is True
+        assert normalized == "FF5733"
+        assert error == ""
+
+    def test_valid_hex_with_prefix_and_lowercase(self):
+        """Test that lowercase hex with # prefix is normalized."""
+        is_valid, normalized, error = validate_hex_color("#ff5733")
+        assert is_valid is True
+        assert normalized == "FF5733"
+        assert error == ""
+
+    def test_empty_string_valid(self):
+        """Test that empty string is valid (means keep current value)."""
+        is_valid, normalized, error = validate_hex_color("")
+        assert is_valid is True
+        assert normalized == ""
+        assert error == ""
+
+    def test_invalid_length_too_short(self):
+        """Test that hex color with wrong length fails validation."""
+        is_valid, normalized, error = validate_hex_color("FF57")
+        assert is_valid is False
+        assert "6 characters" in error
+
+    def test_invalid_length_too_long(self):
+        """Test that hex color with wrong length fails validation."""
+        is_valid, normalized, error = validate_hex_color("FF57331")
+        assert is_valid is False
+        assert "6 characters" in error
+
+    def test_invalid_characters(self):
+        """Test that non-hex characters fail validation."""
+        is_valid, normalized, error = validate_hex_color("GGGGGG")
+        assert is_valid is False
+        assert "hex digits" in error
+
+    def test_invalid_special_characters(self):
+        """Test that special characters fail validation."""
+        is_valid, normalized, error = validate_hex_color("FF57@3")
+        assert is_valid is False
+        assert "hex digits" in error
+
+
+@pytest.mark.asyncio
+class TestImageURLValidation:
+    """Test image URL format and accessibility validation."""
+
+    async def test_valid_png_url(self):
+        """Test that valid PNG URL passes format validation and accessibility check."""
+        url = "https://example.com/logo.png"
+
+        with aioresponses() as m:
+            m.head(url, status=200, headers={'Content-Type': 'image/png'})
+
+            is_valid, error = await validate_image_url(url)
+            assert is_valid is True
+            assert error == ""
+
+    async def test_valid_jpg_url(self):
+        """Test that valid JPG URL passes validation."""
+        url = "https://example.com/logo.jpg"
+
+        with aioresponses() as m:
+            m.head(url, status=200, headers={'Content-Type': 'image/jpeg'})
+
+            is_valid, error = await validate_image_url(url)
+            assert is_valid is True
+            assert error == ""
+
+    async def test_valid_webp_url(self):
+        """Test that valid WebP URL passes validation."""
+        url = "https://example.com/logo.webp"
+
+        with aioresponses() as m:
+            m.head(url, status=200, headers={'Content-Type': 'image/webp'})
+
+            is_valid, error = await validate_image_url(url)
+            assert is_valid is True
+            assert error == ""
+
+    async def test_url_with_query_params(self):
+        """Test that URL with query parameters passes validation."""
+        url = "https://example.com/logo.png?size=large"
+
+        with aioresponses() as m:
+            m.head(url, status=200, headers={'Content-Type': 'image/png'})
+
+            is_valid, error = await validate_image_url(url)
+            assert is_valid is True
+            assert error == ""
+
+    async def test_empty_url_valid(self):
+        """Test that empty URL is valid (means keep current value)."""
+        is_valid, error = await validate_image_url("")
+        assert is_valid is True
+        assert error == ""
+
+    async def test_invalid_protocol_ftp(self):
+        """Test that FTP protocol fails validation."""
+        url = "ftp://example.com/logo.png"
+        is_valid, error = await validate_image_url(url)
+        assert is_valid is False
+        assert "http" in error.lower()
+
+    async def test_invalid_no_protocol(self):
+        """Test that URL without protocol fails validation."""
+        url = "example.com/logo.png"
+        is_valid, error = await validate_image_url(url)
+        assert is_valid is False
+        assert "http" in error.lower()
+
+    async def test_invalid_extension(self):
+        """Test that invalid extension fails validation."""
+        url = "https://example.com/document.pdf"
+        is_valid, error = await validate_image_url(url)
+        assert is_valid is False
+        assert any(ext in error for ext in ['.png', '.jpg', '.jpeg', '.gif', '.webp'])
+
+    async def test_url_not_accessible_404(self):
+        """Test that inaccessible URL (404) fails validation."""
+        url = "https://example.com/logo.png"
+
+        with aioresponses() as m:
+            m.head(url, status=404)
+
+            is_valid, error = await validate_image_url(url)
+            assert is_valid is False
+            assert "404" in error
+
+    async def test_url_wrong_content_type(self):
+        """Test that URL with wrong content-type fails validation."""
+        url = "https://example.com/logo.png"
+
+        with aioresponses() as m:
+            m.head(url, status=200, headers={'Content-Type': 'text/html'})
+
+            is_valid, error = await validate_image_url(url)
+            assert is_valid is False
+            assert "image" in error.lower()
+
+    async def test_url_timeout(self):
+        """Test that timeout fails validation gracefully."""
+        url = "https://example.com/logo.png"
+
+        with aioresponses() as m:
+            m.head(url, exception=asyncio.TimeoutError())
+
+            is_valid, error = await validate_image_url(url)
+            assert is_valid is False
+            assert "timed out" in error.lower()
+
+
+@pytest.mark.asyncio
+class TestBrandingCommand:
+    """Test branding command workflows."""
+
+    @pytest.fixture
+    def mock_bot(self):
+        """Create mock bot instance."""
+        bot = MagicMock()
+        return bot
+
+    @pytest.fixture
+    def branding_cog(self, mock_bot):
+        """Create BrandingCommands cog instance."""
+        return BrandingCommands(mock_bot)
+
+    @pytest.fixture
+    def mock_interaction(self):
+        """Create mock Discord interaction."""
+        interaction = AsyncMock()
+        interaction.user = MagicMock()
+        interaction.user.id = 123456789
+        interaction.guild = MagicMock()
+        interaction.guild.roles = []
+        interaction.response = AsyncMock()
+        interaction.followup = AsyncMock()
+        return interaction
+
+    @pytest.fixture
+    def sample_team(self):
+        """Create sample team data."""
+        return Team(
+            id=1,
+            abbrev="NYY",
+            sname="Yankees",
+            lname="New York Yankees",
+            color="003087",
+            dice_color="E4002B",
+            thumbnail="https://example.com/yankees.png",
+            stadium="Yankee Stadium",
+            season=12,
+            roster_type="majors",
+            owner_id=123456789
+        )
+
+    async def test_validate_all_inputs_major_color_only(self, branding_cog):
+        """Test validating major league color only."""
+        modal_data = {
+            'major_color': 'FF5733',
+            'major_logo': '',
+            'minor_color': '',
+            'minor_logo': '',
+            'dice_color': '',
+        }
+
+        updates, errors = await branding_cog._validate_all_inputs(modal_data)
+
+        assert len(errors) == 0
+        assert updates['major']['color'] == 'FF5733'
+        assert 'thumbnail' not in updates['major']
+        assert len(updates['minor']) == 0
+
+    async def test_validate_all_inputs_dice_color(self, branding_cog):
+        """Test validating dice color update."""
+        modal_data = {
+            'major_color': '',
+            'major_logo': '',
+            'minor_color': '',
+            'minor_logo': '',
+            'dice_color': '#A6CE39',
+        }
+
+        updates, errors = await branding_cog._validate_all_inputs(modal_data)
+
+        assert len(errors) == 0
+        assert updates['major']['dice_color'] == 'A6CE39'
+
+    async def test_validate_all_inputs_invalid_color(self, branding_cog):
+        """Test that invalid color produces error."""
+        modal_data = {
+            'major_color': 'GGGGGG',  # Invalid hex
+            'major_logo': '',
+            'minor_color': '',
+            'minor_logo': '',
+            'dice_color': '',
+        }
+
+        updates, errors = await branding_cog._validate_all_inputs(modal_data)
+
+        assert len(errors) == 1
+        assert "Major Team Color" in errors[0]
+        assert "hex digits" in errors[0]
+
+    async def test_validate_all_inputs_multiple_errors(self, branding_cog):
+        """Test that multiple validation errors are collected."""
+        modal_data = {
+            'major_color': 'GGG',  # Invalid
+            'major_logo': 'not-a-url',  # Invalid
+            'minor_color': '',
+            'minor_logo': '',
+            'dice_color': 'ZZZ',  # Invalid
+        }
+
+        updates, errors = await branding_cog._validate_all_inputs(modal_data)
+
+        assert len(errors) >= 2  # At least color errors
+        assert any("Major Team Color" in e for e in errors)
+        assert any("Dice" in e for e in errors)
+
+    async def test_validate_all_inputs_valid_url(self, branding_cog):
+        """Test that valid URLs are added to updates."""
+        url = "https://example.com/logo.png"
+
+        with aioresponses() as m:
+            m.head(url, status=200, headers={'Content-Type': 'image/png'})
+
+            modal_data = {
+                'major_color': '',
+                'major_logo': url,
+                'minor_color': '',
+                'minor_logo': '',
+                'dice_color': '',
+            }
+
+            updates, errors = await branding_cog._validate_all_inputs(modal_data)
+
+            assert len(errors) == 0
+            assert updates['major']['thumbnail'] == url
+
+    async def test_create_preview_embeds_major_only(self, branding_cog, sample_team):
+        """Test creating preview embeds for major league team only."""
+        updates = {
+            'major': {'color': 'FF5733', 'thumbnail': 'https://example.com/new.png'},
+            'minor': {}
+        }
+
+        embeds = await branding_cog._create_preview_embeds(sample_team, None, updates)
+
+        assert len(embeds) >= 1
+        assert "New York Yankees" in embeds[0].title
+        assert embeds[0].color.value == int('FF5733', 16)
+
+    async def test_create_preview_embeds_with_dice_color(self, branding_cog, sample_team):
+        """Test creating preview embeds including dice color."""
+        updates = {
+            'major': {'dice_color': 'A6CE39'},
+            'minor': {}
+        }
+
+        embeds = await branding_cog._create_preview_embeds(sample_team, None, updates)
+
+        # Should have at least 1 embed (major) and possibly dice embed
+        assert len(embeds) >= 1
+
+    async def test_format_success_message_major_updates(self, branding_cog):
+        """Test formatting success message for major league updates."""
+        updates = {
+            'major': {'color': 'FF5733', 'thumbnail': 'https://example.com/new.png'},
+            'minor': {}
+        }
+
+        message = branding_cog._format_success_message(updates, True, None)
+
+        assert "Major League" in message
+        assert "FF5733" in message
+        assert "Logo" in message
+        assert "✅" in message
+
+    async def test_format_success_message_with_role_error(self, branding_cog):
+        """Test formatting success message when role update fails."""
+        updates = {
+            'major': {'color': 'FF5733'},
+            'minor': {}
+        }
+
+        message = branding_cog._format_success_message(updates, False, "Missing permissions")
+
+        assert "Major League" in message
+        assert "FF5733" in message
+        assert "Missing permissions" in message or "⚠️" in message
+
+    async def test_format_success_message_minor_updates(self, branding_cog):
+        """Test formatting success message for minor league updates."""
+        updates = {
+            'major': {},
+            'minor': {'color': '33C3FF', 'thumbnail': 'https://example.com/mil.png'}
+        }
+
+        message = branding_cog._format_success_message(updates, False, None)
+
+        assert "Minor League" in message
+        assert "33C3FF" in message
+
+
+@pytest.mark.asyncio
+class TestDiscordRoleUpdate:
+    """Test Discord role color update functionality."""
+
+    @pytest.fixture
+    def mock_bot(self):
+        """Create mock bot instance."""
+        return MagicMock()
+
+    @pytest.fixture
+    def branding_cog(self, mock_bot):
+        """Create BrandingCommands cog instance."""
+        return BrandingCommands(mock_bot)
+
+    @pytest.fixture
+    def mock_interaction(self):
+        """Create mock Discord interaction with guild and roles."""
+        interaction = AsyncMock()
+        interaction.user = MagicMock()
+        interaction.user.id = 123456789
+        interaction.guild = MagicMock()
+        interaction.response = AsyncMock()
+        interaction.followup = AsyncMock()
+        return interaction
+
+    @pytest.fixture
+    def sample_team(self):
+        """Create sample team data."""
+        return Team(
+            id=1,
+            abbrev="NYY",
+            sname="Yankees",
+            lname="New York Yankees",
+            color="003087",
+            dice_color="E4002B",
+            thumbnail="https://example.com/yankees.png",
+            stadium="Yankee Stadium",
+            season=12,
+            roster_type="majors",
+            owner_id=123456789
+        )
+
+    async def test_update_discord_role_success(self, branding_cog, mock_interaction, sample_team):
+        """Test successful Discord role color update."""
+        # Create mock role
+        mock_role = AsyncMock()
+        mock_role.name = "New York Yankees"
+        mock_role.edit = AsyncMock()
+        mock_interaction.guild.roles = [mock_role]
+
+        # Patch discord.utils.get to return our mock role
+        with patch('commands.teams.branding.discord.utils.get', return_value=mock_role):
+            success, error = await branding_cog._update_discord_role_color(
+                mock_interaction,
+                sample_team,
+                "FF5733"
+            )
+
+            assert success is True
+            assert error is None
+            mock_role.edit.assert_called_once()
+
+    async def test_update_discord_role_not_found(self, branding_cog, mock_interaction, sample_team):
+        """Test Discord role update when role is not found."""
+        mock_interaction.guild.roles = []
+
+        # Patch discord.utils.get to return None
+        with patch('commands.teams.branding.discord.utils.get', return_value=None):
+            success, error = await branding_cog._update_discord_role_color(
+                mock_interaction,
+                sample_team,
+                "FF5733"
+            )
+
+            assert success is False
+            assert "not found" in error.lower()
+
+    async def test_update_discord_role_forbidden(self, branding_cog, mock_interaction, sample_team):
+        """Test Discord role update when missing permissions."""
+        import discord as discord_module
+
+        # Create mock role that raises Forbidden
+        mock_role = AsyncMock()
+        mock_role.name = "New York Yankees"
+        mock_role.edit = AsyncMock(side_effect=discord_module.Forbidden(
+            MagicMock(status=403), "Missing Permissions"
+        ))
+        mock_interaction.guild.roles = [mock_role]
+
+        with patch('commands.teams.branding.discord.utils.get', return_value=mock_role):
+            success, error = await branding_cog._update_discord_role_color(
+                mock_interaction,
+                sample_team,
+                "FF5733"
+            )
+
+            assert success is False
+            assert "permission" in error.lower()