From 346e36bfc693578d856bbf59ba5362b2b0c34fc1 Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Wed, 4 Mar 2026 22:04:57 -0600 Subject: [PATCH] fix: update roster labels to use Minor League and Injured List (#59) - Add section headers (Active Roster, Minor League, Injured List) to the main summary embed in _create_roster_embeds so each roster section is clearly labeled for users - Fix incorrect docstring in team_service.get_team_roster that had the shortil/longil mapping backwards Co-Authored-By: Claude Sonnet 4.6 --- commands/teams/roster.py | 177 ++++++++++++++++++++++---------------- services/team_service.py | 178 +++++++++++++++++++++------------------ 2 files changed, 202 insertions(+), 153 deletions(-) diff --git a/commands/teams/roster.py b/commands/teams/roster.py index 1acbddc..929996c 100644 --- a/commands/teams/roster.py +++ b/commands/teams/roster.py @@ -1,6 +1,7 @@ """ Team roster commands for Discord Bot v2.0 """ + from typing import Dict, Any, List import discord @@ -18,144 +19,173 @@ from views.embeds import EmbedTemplate, EmbedColors class TeamRosterCommands(commands.Cog): """Team roster command handlers.""" - + def __init__(self, bot: commands.Bot): self.bot = bot - self.logger = get_contextual_logger(f'{__name__}.TeamRosterCommands') + self.logger = get_contextual_logger(f"{__name__}.TeamRosterCommands") self.logger.info("TeamRosterCommands cog initialized") - + @discord.app_commands.command(name="roster", description="Display team roster") @discord.app_commands.describe( abbrev="Team abbreviation (e.g., BSG, DEN, WV, etc.)", - roster_type="Roster week: current or next (defaults to current)" + roster_type="Roster week: current or next (defaults to current)", + ) + @discord.app_commands.choices( + roster_type=[ + discord.app_commands.Choice(name="Current Week", value="current"), + discord.app_commands.Choice(name="Next Week", value="next"), + ] ) - @discord.app_commands.choices(roster_type=[ - discord.app_commands.Choice(name="Current Week", value="current"), - discord.app_commands.Choice(name="Next Week", value="next") - ]) @requires_team() @logged_command("/roster") - async def team_roster(self, interaction: discord.Interaction, abbrev: str, - roster_type: str = "current"): + async def team_roster( + self, + interaction: discord.Interaction, + abbrev: str, + roster_type: str = "current", + ): """Display team roster with position breakdowns.""" await interaction.response.defer() - + # Get team by abbreviation team = await team_service.get_team_by_abbrev(abbrev, get_config().sba_season) - + if team is None: self.logger.info("Team not found", team_abbrev=abbrev) embed = EmbedTemplate.error( title="Team Not Found", - description=f"No team found with abbreviation '{abbrev.upper()}'" + description=f"No team found with abbreviation '{abbrev.upper()}'", ) await interaction.followup.send(embed=embed) return - + # Get roster data roster_data = await team_service.get_team_roster(team.id, roster_type) - + if not roster_data: embed = EmbedTemplate.error( title="Roster Not Available", - description=f"No {roster_type} roster data available for {team.abbrev}" + description=f"No {roster_type} roster data available for {team.abbrev}", ) await interaction.followup.send(embed=embed) return - + # Create roster embeds embeds = await self._create_roster_embeds(team, roster_data, roster_type) - + # Send first embed and follow up with others if needed await interaction.followup.send(embed=embeds[0]) for embed in embeds[1:]: await interaction.followup.send(embed=embed) - - async def _create_roster_embeds(self, team: Team, roster_data: Dict[str, Any], - roster_type: str) -> List[discord.Embed]: + + async def _create_roster_embeds( + self, team: Team, roster_data: Dict[str, Any], roster_type: str + ) -> List[discord.Embed]: """Create embeds for team roster data.""" embeds = [] - + # Main roster embed embed = EmbedTemplate.create_base_embed( title=f"{team.abbrev} - {roster_type.title()} Week", description=f"{team.lname} Roster Breakdown", - color=int(team.color, 16) if team.color else EmbedColors.PRIMARY + color=int(team.color, 16) if team.color else EmbedColors.PRIMARY, ) - + # Position counts for active roster - for key in ['active', 'longil', 'shortil']: - if key in roster_data: + roster_titles = { + "active": "Active Roster", + "longil": "Minor League", + "shortil": "Injured List", + } + for key in ["active", "longil", "shortil"]: + if key in roster_data: this_roster = roster_data[key] - players = this_roster.get('players') - if len(players) > 0: - this_team = players[0].get("team", {"id": "Unknown", "sname": "Unknown"}) + embed.add_field(name=roster_titles[key], value="\u200b", inline=False) - embed.add_field( - name='Team (ID)', - value=f'{this_team.get("sname")} ({this_team.get("id")})', - inline=True + players = this_roster.get("players") + if len(players) > 0: + this_team = players[0].get( + "team", {"id": "Unknown", "sname": "Unknown"} ) embed.add_field( - name='Player Count', - value=f'{len(players)} Players' + name="Team (ID)", + value=f'{this_team.get("sname")} ({this_team.get("id")})', + inline=True, + ) + + embed.add_field( + name="Player Count", value=f"{len(players)} Players" ) # Total WAR - total_war = this_roster.get('WARa', 0) + total_war = this_roster.get("WARa", 0) embed.add_field( - name="Total sWAR", - value=f"{total_war:.2f}" if isinstance(total_war, (int, float)) else str(total_war), - inline=True + name="Total sWAR", + value=( + f"{total_war:.2f}" + if isinstance(total_war, (int, float)) + else str(total_war) + ), + inline=True, ) - + embed.add_field( - name='Position Counts', + name="Position Counts", value=self._position_code_block(this_roster), - inline=False + inline=False, ) - + embeds.append(embed) - + # Create detailed player list embeds if there are players for roster_name, roster_info in roster_data.items(): - if roster_name in ['active', 'longil', 'shortil'] and 'players' in roster_info: - players = sorted(roster_info['players'], key=lambda player: player.get('wara', 0), reverse=True) + if ( + roster_name in ["active", "longil", "shortil"] + and "players" in roster_info + ): + players = sorted( + roster_info["players"], + key=lambda player: player.get("wara", 0), + reverse=True, + ) if players: player_embed = self._create_player_list_embed( team, roster_name, players ) embeds.append(player_embed) - - return embeds - - def _position_code_block(self, roster_data: dict) -> str: - return f'```\n C 1B 2B 3B SS\n' \ - f' {roster_data.get("C", 0)} {roster_data.get("1B", 0)} {roster_data.get("2B", 0)} ' \ - f'{roster_data.get("3B", 0)} {roster_data.get("SS", 0)}\n\nLF CF RF SP RP\n' \ - f' {roster_data.get("LF", 0)} {roster_data.get("CF", 0)} {roster_data.get("RF", 0)} ' \ - f'{roster_data.get("SP", 0)} {roster_data.get("RP", 0)}\n```' - def _create_player_list_embed(self, team: Team, roster_name: str, - players: List[Dict[str, Any]]) -> discord.Embed: + return embeds + + def _position_code_block(self, roster_data: dict) -> str: + return ( + f"```\n C 1B 2B 3B SS\n" + f' {roster_data.get("C", 0)} {roster_data.get("1B", 0)} {roster_data.get("2B", 0)} ' + f'{roster_data.get("3B", 0)} {roster_data.get("SS", 0)}\n\nLF CF RF SP RP\n' + f' {roster_data.get("LF", 0)} {roster_data.get("CF", 0)} {roster_data.get("RF", 0)} ' + f'{roster_data.get("SP", 0)} {roster_data.get("RP", 0)}\n```' + ) + + def _create_player_list_embed( + self, team: Team, roster_name: str, players: List[Dict[str, Any]] + ) -> discord.Embed: """Create an embed with detailed player list.""" roster_titles = { - 'active': 'Active Roster', - 'longil': 'Minor League', - 'shortil': 'Injured List' + "active": "Active Roster", + "longil": "Minor League", + "shortil": "Injured List", } - + embed = EmbedTemplate.create_base_embed( title=f"{team.abbrev} - {roster_titles.get(roster_name, roster_name.title())}", - color=int(team.color, 16) if team.color else EmbedColors.PRIMARY + color=int(team.color, 16) if team.color else EmbedColors.PRIMARY, ) - + # Group players by position for better organization batters = [] pitchers = [] - + for player in players: try: this_player = Player.from_api_data(player) @@ -166,8 +196,11 @@ class TeamRosterCommands(commands.Cog): else: batters.append(player_line) except Exception as e: - self.logger.warning(f"Failed to create player from data: {e}", player_id=player.get('id')) - + self.logger.warning( + f"Failed to create player from data: {e}", + player_id=player.get("id"), + ) + # Add player lists to embed if batters: # Split long lists into multiple fields if needed @@ -175,18 +208,18 @@ class TeamRosterCommands(commands.Cog): for i, chunk in enumerate(batter_chunks): field_name = "Batters" if i == 0 else f"Batters (cont.)" embed.add_field(name=field_name, value="\n".join(chunk), inline=True) - embed.add_field(name='', value='', inline=False) - + embed.add_field(name="", value="", inline=False) + if pitchers: pitcher_chunks = self._chunk_list(pitchers, 16) for i, chunk in enumerate(pitcher_chunks): field_name = "Pitchers" if i == 0 else f"Pitchers (cont.)" embed.add_field(name=field_name, value="\n".join(chunk), inline=False) - + embed.set_footer(text=f"Total players: {len(players)}") - + return embed - + def _chunk_list(self, lst: List[str], chunk_size: int) -> List[List[str]]: """Split a list into chunks of specified size.""" - return [lst[i:i + chunk_size] for i in range(0, len(lst), chunk_size)] \ No newline at end of file + return [lst[i : i + chunk_size] for i in range(0, len(lst), chunk_size)] diff --git a/services/team_service.py b/services/team_service.py index aa824bd..9bbd555 100644 --- a/services/team_service.py +++ b/services/team_service.py @@ -3,6 +3,7 @@ Team service for Discord Bot v2.0 Handles team-related operations with roster management and league queries. """ + import logging from typing import Optional, List, Dict, Any @@ -12,13 +13,13 @@ from models.team import Team, RosterType from exceptions import APIException from utils.decorators import cached_single_item -logger = logging.getLogger(f'{__name__}.TeamService') +logger = logging.getLogger(f"{__name__}.TeamService") class TeamService(BaseService[Team]): """ Service for team-related operations. - + Features: - Team retrieval by ID, abbreviation, and season - Manager-based team queries @@ -27,12 +28,12 @@ class TeamService(BaseService[Team]): - Season-specific team data - Standings integration """ - + def __init__(self): """Initialize team service.""" - super().__init__(Team, 'teams') + super().__init__(Team, "teams") logger.debug("TeamService initialized") - + @cached_single_item(ttl=1800) # 30-minute cache async def get_team(self, team_id: int) -> Optional[Team]: """ @@ -57,12 +58,12 @@ class TeamService(BaseService[Team]): except Exception as e: logger.error(f"Unexpected error getting team {team_id}: {e}") return None - + async def get_teams_by_owner( self, owner_id: int, season: Optional[int] = None, - roster_type: Optional[str] = None + roster_type: Optional[str] = None, ) -> List[Team]: """ Get teams owned by a specific Discord user. @@ -80,10 +81,7 @@ class TeamService(BaseService[Team]): Allows caller to distinguish between "no teams" vs "error occurred" """ season = season or get_config().sba_season - params = [ - ('owner_id', str(owner_id)), - ('season', str(season)) - ] + params = [("owner_id", str(owner_id)), ("season", str(season))] teams = await self.get_all_items(params=params) @@ -92,19 +90,27 @@ class TeamService(BaseService[Team]): try: target_type = RosterType(roster_type) teams = [team for team in teams if team.roster_type() == target_type] - logger.debug(f"Filtered to {len(teams)} {roster_type} teams for owner {owner_id}") + logger.debug( + f"Filtered to {len(teams)} {roster_type} teams for owner {owner_id}" + ) except ValueError: - logger.warning(f"Invalid roster_type '{roster_type}' - returning all teams") + logger.warning( + f"Invalid roster_type '{roster_type}' - returning all teams" + ) if teams: - logger.debug(f"Found {len(teams)} teams for owner {owner_id} in season {season}") + logger.debug( + f"Found {len(teams)} teams for owner {owner_id} in season {season}" + ) return teams logger.debug(f"No teams found for owner {owner_id} in season {season}") return [] @cached_single_item(ttl=1800) # 30-minute cache - async def get_team_by_owner(self, owner_id: int, season: Optional[int] = None) -> Optional[Team]: + async def get_team_by_owner( + self, owner_id: int, season: Optional[int] = None + ) -> Optional[Team]: """ Get the primary (Major League) team owned by a Discord user. @@ -124,125 +130,129 @@ class TeamService(BaseService[Team]): Returns: Team instance or None if not found """ - teams = await self.get_teams_by_owner(owner_id, season, roster_type='ml') + teams = await self.get_teams_by_owner(owner_id, season, roster_type="ml") return teams[0] if teams else None - async def get_team_by_abbrev(self, abbrev: str, season: Optional[int] = None) -> Optional[Team]: + async def get_team_by_abbrev( + self, abbrev: str, season: Optional[int] = None + ) -> Optional[Team]: """ Get team by abbreviation for a specific season. - + Args: abbrev: Team abbreviation (e.g., 'NYY', 'BOS') season: Season number (defaults to current season) - + Returns: Team instance or None if not found """ try: season = season or get_config().sba_season - params = [ - ('team_abbrev', abbrev.upper()), - ('season', str(season)) - ] - + params = [("team_abbrev", abbrev.upper()), ("season", str(season))] + teams = await self.get_all_items(params=params) - + if teams: team = teams[0] # Should be unique per season logger.debug(f"Found team {abbrev} for season {season}: {team.lname}") return team - - logger.debug(f"No team found for abbreviation '{abbrev}' in season {season}") + + logger.debug( + f"No team found for abbreviation '{abbrev}' in season {season}" + ) return None - + except Exception as e: logger.error(f"Error getting team by abbreviation '{abbrev}': {e}") return None - + async def get_teams_by_season(self, season: int) -> List[Team]: """ Get all teams for a specific season. - + Args: season: Season number - + Returns: List of teams in the season """ try: - params = [('season', str(season))] - + params = [("season", str(season))] + teams = await self.get_all_items(params=params) logger.debug(f"Retrieved {len(teams)} teams for season {season}") return teams - + except Exception as e: logger.error(f"Failed to get teams for season {season}: {e}") return [] - - async def get_teams_by_manager(self, manager_id: int, season: Optional[int] = None) -> List[Team]: + + async def get_teams_by_manager( + self, manager_id: int, season: Optional[int] = None + ) -> List[Team]: """ Get teams managed by a specific manager. - + Uses 'manager_id' query parameter which supports multiple manager matching. - + Args: manager_id: Manager identifier season: Season number (optional) - + Returns: List of teams managed by the manager """ try: - params = [('manager_id', str(manager_id))] - + params = [("manager_id", str(manager_id))] + if season: - params.append(('season', str(season))) - + params.append(("season", str(season))) + teams = await self.get_all_items(params=params) logger.debug(f"Found {len(teams)} teams for manager {manager_id}") return teams - + except Exception as e: logger.error(f"Failed to get teams for manager {manager_id}: {e}") return [] - + async def get_teams_by_division(self, division_id: int, season: int) -> List[Team]: """ Get teams in a specific division for a season. - + Args: division_id: Division identifier season: Season number - + Returns: List of teams in the division """ try: - params = [ - ('division_id', str(division_id)), - ('season', str(season)) - ] - + params = [("division_id", str(division_id)), ("season", str(season))] + teams = await self.get_all_items(params=params) - logger.debug(f"Retrieved {len(teams)} teams for division {division_id} in season {season}") + logger.debug( + f"Retrieved {len(teams)} teams for division {division_id} in season {season}" + ) return teams - + except Exception as e: logger.error(f"Failed to get teams for division {division_id}: {e}") return [] - - async def get_team_roster(self, team_id: int, roster_type: str = 'current') -> Optional[Dict[str, Any]]: + + async def get_team_roster( + self, team_id: int, roster_type: str = "current" + ) -> Optional[Dict[str, Any]]: """ Get the roster for a team with position counts and player lists. - - Returns roster data with active, shortil (minor league), and longil (injured list) + + Returns roster data with active, shortil (injured list), and longil (minor league) rosters. Each roster contains position counts and players sorted by descending WARa. - + Args: team_id: Team identifier roster_type: 'current' or 'next' roster - + Returns: Dictionary with roster structure: { @@ -257,19 +267,19 @@ class TeamService(BaseService[Team]): """ try: client = await self.get_client() - data = await client.get(f'teams/{team_id}/roster/{roster_type}') - + data = await client.get(f"teams/{team_id}/roster/{roster_type}") + if data: logger.debug(f"Retrieved {roster_type} roster for team {team_id}") return data - + logger.debug(f"No roster data found for team {team_id}") return None - + except Exception as e: logger.error(f"Failed to get roster for team {team_id}: {e}") return None - + async def update_team(self, team_id: int, updates: dict) -> Optional[Team]: """ Update team information. @@ -287,52 +297,58 @@ class TeamService(BaseService[Team]): except Exception as e: logger.error(f"Failed to update team {team_id}: {e}") return None - - async def get_team_standings_position(self, team_id: int, season: int) -> Optional[dict]: + + async def get_team_standings_position( + self, team_id: int, season: int + ) -> Optional[dict]: """ Get team's standings information. - + Calls /standings/team/{team_id} endpoint which returns a Standings object. - + Args: team_id: Team identifier season: Season number - + Returns: Standings object data for the team """ try: client = await self.get_client() - data = await client.get(f'standings/team/{team_id}', params=[('season', str(season))]) - + data = await client.get( + f"standings/team/{team_id}", params=[("season", str(season))] + ) + if data: logger.debug(f"Retrieved standings for team {team_id}") return data - + return None - + except Exception as e: logger.error(f"Failed to get standings for team {team_id}: {e}") return None - - async def is_valid_team_abbrev(self, abbrev: str, season: Optional[int] = None) -> bool: + + async def is_valid_team_abbrev( + self, abbrev: str, season: Optional[int] = None + ) -> bool: """ Check if a team abbreviation is valid for a season. - + Args: abbrev: Team abbreviation to validate season: Season number (defaults to current) - + Returns: True if the abbreviation is valid """ team = await self.get_team_by_abbrev(abbrev, season) return team is not None - + async def get_current_season_teams(self) -> List[Team]: """ Get all teams for the current season. - + Returns: List of teams in current season """ @@ -340,4 +356,4 @@ class TeamService(BaseService[Team]): # Global service instance -team_service = TeamService() \ No newline at end of file +team_service = TeamService()