From 58043c996d8966138c17165acfa4d2cf96400ba7 Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Sun, 1 Mar 2026 23:29:01 -0600 Subject: [PATCH] fix: auto-detect player roster type in trade commands instead of assuming ML /trade add-player hardcoded from_roster=MAJOR_LEAGUE for all players, causing incorrect transaction records when trading MiL/IL players. This cascaded into false roster validation errors for other teams' /dropadd commands because pre-existing transaction processing misidentified which roster a player was leaving from. Now looks up the player's actual team and calls roster_type() to determine the correct source roster. Same fix applied to /trade supplementary. Co-Authored-By: Claude Opus 4.6 --- commands/transactions/trade.py | 256 ++++++++++++++++----------------- 1 file changed, 122 insertions(+), 134 deletions(-) diff --git a/commands/transactions/trade.py b/commands/transactions/trade.py index f937152..64bb23f 100644 --- a/commands/transactions/trade.py +++ b/commands/transactions/trade.py @@ -3,6 +3,7 @@ Trade Commands Interactive multi-team trade builder with real-time validation and elegant UX. """ + from typing import Optional import discord @@ -12,7 +13,11 @@ from discord import app_commands from config import get_config from utils.logging import get_contextual_logger from utils.decorators import logged_command -from utils.autocomplete import player_autocomplete, major_league_team_autocomplete, team_autocomplete +from utils.autocomplete import ( + player_autocomplete, + major_league_team_autocomplete, + team_autocomplete, +) from utils.team_utils import validate_user_has_team, get_team_by_abbrev_with_validation from services.trade_builder import ( @@ -22,6 +27,7 @@ from services.trade_builder import ( clear_trade_builder_by_team, ) from services.player_service import player_service +from services.team_service import team_service from models.team import RosterType from views.trade_embed import TradeEmbedView, create_trade_embed from commands.transactions.trade_channels import TradeChannelManager @@ -33,16 +39,20 @@ class TradeCommands(commands.Cog): def __init__(self, bot: commands.Bot): self.bot = bot - self.logger = get_contextual_logger(f'{__name__}.TradeCommands') + self.logger = get_contextual_logger(f"{__name__}.TradeCommands") # Initialize trade channel management self.channel_tracker = TradeChannelTracker() self.channel_manager = TradeChannelManager(self.channel_tracker) # Create the trade command group - trade_group = app_commands.Group(name="trade", description="Multi-team trade management") + trade_group = app_commands.Group( + name="trade", description="Multi-team trade management" + ) - def _get_trade_channel(self, guild: discord.Guild, trade_id: str) -> Optional[discord.TextChannel]: + def _get_trade_channel( + self, guild: discord.Guild, trade_id: str + ) -> Optional[discord.TextChannel]: """Get the trade channel for a given trade ID.""" channel_data = self.channel_tracker.get_channel_by_trade_id(trade_id) if not channel_data: @@ -55,7 +65,9 @@ class TradeCommands(commands.Cog): return channel return None - def _is_in_trade_channel(self, interaction: discord.Interaction, trade_id: str) -> bool: + def _is_in_trade_channel( + self, interaction: discord.Interaction, trade_id: str + ) -> bool: """Check if the interaction is happening in the trade's dedicated channel.""" trade_channel = self._get_trade_channel(interaction.guild, trade_id) if not trade_channel: @@ -68,7 +80,7 @@ class TradeCommands(commands.Cog): trade_id: str, embed: discord.Embed, view: Optional[discord.ui.View] = None, - content: Optional[str] = None + content: Optional[str] = None, ) -> bool: """ Post the trade embed to the trade channel. @@ -90,19 +102,12 @@ class TradeCommands(commands.Cog): return False @trade_group.command( - name="initiate", - description="Start a new trade with another team" - ) - @app_commands.describe( - other_team="Team abbreviation to trade with" + name="initiate", description="Start a new trade with another team" ) + @app_commands.describe(other_team="Team abbreviation to trade with") @app_commands.autocomplete(other_team=major_league_team_autocomplete) @logged_command("/trade initiate") - async def trade_initiate( - self, - interaction: discord.Interaction, - other_team: str - ): + async def trade_initiate(self, interaction: discord.Interaction, other_team: str): """Initiate a new trade with another team.""" await interaction.response.defer(ephemeral=True) @@ -112,15 +117,16 @@ class TradeCommands(commands.Cog): return # Get the other team - other_team_obj = await get_team_by_abbrev_with_validation(other_team, interaction) + other_team_obj = await get_team_by_abbrev_with_validation( + other_team, interaction + ) if not other_team_obj: return # Check if it's the same team if user_team.id == other_team_obj.id: await interaction.followup.send( - "āŒ You cannot initiate a trade with yourself.", - ephemeral=True + "āŒ You cannot initiate a trade with yourself.", ephemeral=True ) return @@ -133,7 +139,7 @@ class TradeCommands(commands.Cog): if not success: await interaction.followup.send( f"āŒ Failed to add {other_team_obj.abbrev} to trade: {error_msg}", - ephemeral=True + ephemeral=True, ) return @@ -143,7 +149,7 @@ class TradeCommands(commands.Cog): trade_id=trade_builder.trade_id, team1=user_team, team2=other_team_obj, - creator_id=interaction.user.id + creator_id=interaction.user.id, ) # Show trade interface @@ -156,31 +162,26 @@ class TradeCommands(commands.Cog): success_msg += f"\nšŸ“ Discussion channel: {channel.mention}" else: success_msg += f"\nāš ļø **Warning:** Failed to create discussion channel. Check bot permissions or contact an admin." - self.logger.warning(f"Failed to create trade channel for trade {trade_builder.trade_id}") + self.logger.warning( + f"Failed to create trade channel for trade {trade_builder.trade_id}" + ) await interaction.followup.send( - content=success_msg, - embed=embed, - view=view, - ephemeral=True + content=success_msg, embed=embed, view=view, ephemeral=True ) - self.logger.info(f"Trade initiated: {user_team.abbrev} ā†” {other_team_obj.abbrev}") + self.logger.info( + f"Trade initiated: {user_team.abbrev} ā†” {other_team_obj.abbrev}" + ) @trade_group.command( name="add-team", - description="Add another team to your current trade (for 3+ team trades)" - ) - @app_commands.describe( - other_team="Team abbreviation to add to the trade" + description="Add another team to your current trade (for 3+ team trades)", ) + @app_commands.describe(other_team="Team abbreviation to add to the trade") @app_commands.autocomplete(other_team=major_league_team_autocomplete) @logged_command("/trade add-team") - async def trade_add_team( - self, - interaction: discord.Interaction, - other_team: str - ): + async def trade_add_team(self, interaction: discord.Interaction, other_team: str): """Add a team to an existing trade.""" await interaction.response.defer(ephemeral=False) @@ -194,7 +195,7 @@ class TradeCommands(commands.Cog): if not trade_builder: await interaction.followup.send( "āŒ Your team is not part of an active trade. Use `/trade initiate` first.", - ephemeral=True + ephemeral=True, ) return @@ -207,8 +208,7 @@ class TradeCommands(commands.Cog): success, error_msg = await trade_builder.add_team(team_to_add) if not success: await interaction.followup.send( - f"āŒ Failed to add {team_to_add.abbrev}: {error_msg}", - ephemeral=True + f"āŒ Failed to add {team_to_add.abbrev}: {error_msg}", ephemeral=True ) return @@ -216,7 +216,7 @@ class TradeCommands(commands.Cog): channel_updated = await self.channel_manager.add_team_to_channel( guild=interaction.guild, trade_id=trade_builder.trade_id, - new_team=team_to_add + new_team=team_to_add, ) # Show updated trade interface @@ -226,13 +226,12 @@ class TradeCommands(commands.Cog): # Build success message success_msg = f"āœ… **Added {team_to_add.abbrev} to the trade**" if channel_updated: - success_msg += f"\nšŸ“ {team_to_add.abbrev} has been added to the discussion channel" + success_msg += ( + f"\nšŸ“ {team_to_add.abbrev} has been added to the discussion channel" + ) await interaction.followup.send( - content=success_msg, - embed=embed, - view=view, - ephemeral=True + content=success_msg, embed=embed, view=view, ephemeral=True ) # If command was executed outside trade channel, post update to trade channel @@ -242,27 +241,23 @@ class TradeCommands(commands.Cog): trade_id=trade_builder.trade_id, embed=embed, view=view, - content=success_msg + content=success_msg, ) - self.logger.info(f"Team added to trade {trade_builder.trade_id}: {team_to_add.abbrev}") + self.logger.info( + f"Team added to trade {trade_builder.trade_id}: {team_to_add.abbrev}" + ) - @trade_group.command( - name="add-player", - description="Add a player to the trade" - ) + @trade_group.command(name="add-player", description="Add a player to the trade") @app_commands.describe( player_name="Player name; begin typing for autocomplete", - destination_team="Team abbreviation where the player will go" + destination_team="Team abbreviation where the player will go", ) @app_commands.autocomplete(player_name=player_autocomplete) @app_commands.autocomplete(destination_team=team_autocomplete) @logged_command("/trade add-player") async def trade_add_player( - self, - interaction: discord.Interaction, - player_name: str, - destination_team: str + self, interaction: discord.Interaction, player_name: str, destination_team: str ): """Add a player move to the trade.""" await interaction.response.defer(ephemeral=False) @@ -277,16 +272,17 @@ class TradeCommands(commands.Cog): if not trade_builder: await interaction.followup.send( "āŒ Your team is not part of an active trade. Use `/trade initiate` or ask another GM to add your team.", - ephemeral=True + ephemeral=True, ) return # Find the player - players = await player_service.search_players(player_name, limit=10, season=get_config().sba_season) + players = await player_service.search_players( + player_name, limit=10, season=get_config().sba_season + ) if not players: await interaction.followup.send( - f"āŒ Player '{player_name}' not found.", - ephemeral=True + f"āŒ Player '{player_name}' not found.", ephemeral=True ) return @@ -300,15 +296,19 @@ class TradeCommands(commands.Cog): player = players[0] # Get destination team - dest_team = await get_team_by_abbrev_with_validation(destination_team, interaction) + dest_team = await get_team_by_abbrev_with_validation( + destination_team, interaction + ) if not dest_team: return - # Determine source team and roster locations - # For now, assume player comes from user's team and goes to ML of destination - # The service will validate that the player is actually on the user's team organization - from_roster = RosterType.MAJOR_LEAGUE # Default assumption - to_roster = RosterType.MAJOR_LEAGUE # Default destination + # Auto-detect source roster from player's actual team assignment + player_team = await team_service.get_team(player.team_id) + if player_team: + from_roster = player_team.roster_type() + else: + from_roster = RosterType.MAJOR_LEAGUE # Fallback + to_roster = dest_team.roster_type() # Add the player move (service layer will validate) success, error_msg = await trade_builder.add_player_move( @@ -316,26 +316,22 @@ class TradeCommands(commands.Cog): from_team=user_team, to_team=dest_team, from_roster=from_roster, - to_roster=to_roster + to_roster=to_roster, ) if not success: - await interaction.followup.send( - f"āŒ {error_msg}", - ephemeral=True - ) + await interaction.followup.send(f"āŒ {error_msg}", ephemeral=True) return # Show updated trade interface embed = await create_trade_embed(trade_builder) view = TradeEmbedView(trade_builder, interaction.user.id) - success_msg = f"āœ… **Added {player.name}: {user_team.abbrev} ā†’ {dest_team.abbrev}**" + success_msg = ( + f"āœ… **Added {player.name}: {user_team.abbrev} ā†’ {dest_team.abbrev}**" + ) await interaction.followup.send( - content=success_msg, - embed=embed, - view=view, - ephemeral=True + content=success_msg, embed=embed, view=view, ephemeral=True ) # If command was executed outside trade channel, post update to trade channel @@ -345,31 +341,32 @@ class TradeCommands(commands.Cog): trade_id=trade_builder.trade_id, embed=embed, view=view, - content=success_msg + content=success_msg, ) - self.logger.info(f"Player added to trade {trade_builder.trade_id}: {player.name} to {dest_team.abbrev}") + self.logger.info( + f"Player added to trade {trade_builder.trade_id}: {player.name} to {dest_team.abbrev}" + ) @trade_group.command( name="supplementary", - description="Add a supplementary move within your organization for roster legality" + description="Add a supplementary move within your organization for roster legality", ) @app_commands.describe( player_name="Player name; begin typing for autocomplete", - destination="Where to move the player: Major League, Minor League, or Free Agency" + destination="Where to move the player: Major League, Minor League, or Free Agency", ) @app_commands.autocomplete(player_name=player_autocomplete) - @app_commands.choices(destination=[ - app_commands.Choice(name="Major League", value="ml"), - app_commands.Choice(name="Minor League", value="mil"), - app_commands.Choice(name="Free Agency", value="fa") - ]) + @app_commands.choices( + destination=[ + app_commands.Choice(name="Major League", value="ml"), + app_commands.Choice(name="Minor League", value="mil"), + app_commands.Choice(name="Free Agency", value="fa"), + ] + ) @logged_command("/trade supplementary") async def trade_supplementary( - self, - interaction: discord.Interaction, - player_name: str, - destination: str + self, interaction: discord.Interaction, player_name: str, destination: str ): """Add a supplementary (internal organization) move for roster legality.""" await interaction.response.defer(ephemeral=False) @@ -384,16 +381,17 @@ class TradeCommands(commands.Cog): if not trade_builder: await interaction.followup.send( "āŒ Your team is not part of an active trade. Use `/trade initiate` or ask another GM to add your team.", - ephemeral=True + ephemeral=True, ) return # Find the player - players = await player_service.search_players(player_name, limit=10, season=get_config().sba_season) + players = await player_service.search_players( + player_name, limit=10, season=get_config().sba_season + ) if not players: await interaction.followup.send( - f"āŒ Player '{player_name}' not found.", - ephemeral=True + f"āŒ Player '{player_name}' not found.", ephemeral=True ) return @@ -403,45 +401,47 @@ class TradeCommands(commands.Cog): destination_map = { "ml": RosterType.MAJOR_LEAGUE, "mil": RosterType.MINOR_LEAGUE, - "fa": RosterType.FREE_AGENCY + "fa": RosterType.FREE_AGENCY, } to_roster = destination_map.get(destination.lower()) if not to_roster: await interaction.followup.send( - f"āŒ Invalid destination: {destination}", - ephemeral=True + f"āŒ Invalid destination: {destination}", ephemeral=True ) return - # Determine current roster (default assumption) - from_roster = RosterType.MINOR_LEAGUE if to_roster == RosterType.MAJOR_LEAGUE else RosterType.MAJOR_LEAGUE + # Auto-detect source roster from player's actual team assignment + player_team = await team_service.get_team(player.team_id) + if player_team: + from_roster = player_team.roster_type() + else: + from_roster = ( + RosterType.MINOR_LEAGUE + if to_roster == RosterType.MAJOR_LEAGUE + else RosterType.MAJOR_LEAGUE + ) # Add supplementary move success, error_msg = await trade_builder.add_supplementary_move( - team=user_team, - player=player, - from_roster=from_roster, - to_roster=to_roster + team=user_team, player=player, from_roster=from_roster, to_roster=to_roster ) if not success: await interaction.followup.send( - f"āŒ Failed to add supplementary move: {error_msg}", - ephemeral=True + f"āŒ Failed to add supplementary move: {error_msg}", ephemeral=True ) return # Show updated trade interface embed = await create_trade_embed(trade_builder) view = TradeEmbedView(trade_builder, interaction.user.id) - success_msg = f"āœ… **Added supplementary move: {player.name} ā†’ {destination.upper()}**" + success_msg = ( + f"āœ… **Added supplementary move: {player.name} ā†’ {destination.upper()}**" + ) await interaction.followup.send( - content=success_msg, - embed=embed, - view=view, - ephemeral=True + content=success_msg, embed=embed, view=view, ephemeral=True ) # If command was executed outside trade channel, post update to trade channel @@ -451,15 +451,14 @@ class TradeCommands(commands.Cog): trade_id=trade_builder.trade_id, embed=embed, view=view, - content=success_msg + content=success_msg, ) - self.logger.info(f"Supplementary move added to trade {trade_builder.trade_id}: {player.name} to {destination}") + self.logger.info( + f"Supplementary move added to trade {trade_builder.trade_id}: {player.name} to {destination}" + ) - @trade_group.command( - name="view", - description="View your current trade" - ) + @trade_group.command(name="view", description="View your current trade") @logged_command("/trade view") async def trade_view(self, interaction: discord.Interaction): """View the current trade.""" @@ -474,8 +473,7 @@ class TradeCommands(commands.Cog): trade_builder = get_trade_builder_by_team(user_team.id) if not trade_builder: await interaction.followup.send( - "āŒ Your team is not part of an active trade.", - ephemeral=True + "āŒ Your team is not part of an active trade.", ephemeral=True ) return @@ -483,11 +481,7 @@ class TradeCommands(commands.Cog): embed = await create_trade_embed(trade_builder) view = TradeEmbedView(trade_builder, interaction.user.id) - await interaction.followup.send( - embed=embed, - view=view, - ephemeral=True - ) + await interaction.followup.send(embed=embed, view=view, ephemeral=True) # If command was executed outside trade channel, post update to trade channel if not self._is_in_trade_channel(interaction, trade_builder.trade_id): @@ -495,13 +489,10 @@ class TradeCommands(commands.Cog): guild=interaction.guild, trade_id=trade_builder.trade_id, embed=embed, - view=view + view=view, ) - @trade_group.command( - name="clear", - description="Clear your current trade" - ) + @trade_group.command(name="clear", description="Clear your current trade") @logged_command("/trade clear") async def trade_clear(self, interaction: discord.Interaction): """Clear the current trade.""" @@ -516,8 +507,7 @@ class TradeCommands(commands.Cog): trade_builder = get_trade_builder_by_team(user_team.id) if not trade_builder: await interaction.followup.send( - "āŒ Your team is not part of an active trade.", - ephemeral=True + "āŒ Your team is not part of an active trade.", ephemeral=True ) return @@ -525,19 +515,17 @@ class TradeCommands(commands.Cog): # Delete associated trade channel if it exists await self.channel_manager.delete_trade_channel( - guild=interaction.guild, - trade_id=trade_id + guild=interaction.guild, trade_id=trade_id ) # Clear the trade builder using team-based function clear_trade_builder_by_team(user_team.id) await interaction.followup.send( - "āœ… The trade has been cleared.", - ephemeral=True + "āœ… The trade has been cleared.", ephemeral=True ) async def setup(bot): """Setup function for the cog.""" - await bot.add_cog(TradeCommands(bot)) \ No newline at end of file + await bot.add_cog(TradeCommands(bot))