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 <noreply@anthropic.com>
This commit is contained in:
Cal Corum 2026-03-01 23:29:01 -06:00
parent f7a65706a1
commit 58043c996d

View File

@ -3,6 +3,7 @@ Trade Commands
Interactive multi-team trade builder with real-time validation and elegant UX. Interactive multi-team trade builder with real-time validation and elegant UX.
""" """
from typing import Optional from typing import Optional
import discord import discord
@ -12,7 +13,11 @@ from discord import app_commands
from config import get_config from config import get_config
from utils.logging import get_contextual_logger from utils.logging import get_contextual_logger
from utils.decorators import logged_command 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 utils.team_utils import validate_user_has_team, get_team_by_abbrev_with_validation
from services.trade_builder import ( from services.trade_builder import (
@ -22,6 +27,7 @@ from services.trade_builder import (
clear_trade_builder_by_team, clear_trade_builder_by_team,
) )
from services.player_service import player_service from services.player_service import player_service
from services.team_service import team_service
from models.team import RosterType from models.team import RosterType
from views.trade_embed import TradeEmbedView, create_trade_embed from views.trade_embed import TradeEmbedView, create_trade_embed
from commands.transactions.trade_channels import TradeChannelManager from commands.transactions.trade_channels import TradeChannelManager
@ -33,16 +39,20 @@ class TradeCommands(commands.Cog):
def __init__(self, bot: commands.Bot): def __init__(self, bot: commands.Bot):
self.bot = bot self.bot = bot
self.logger = get_contextual_logger(f'{__name__}.TradeCommands') self.logger = get_contextual_logger(f"{__name__}.TradeCommands")
# Initialize trade channel management # Initialize trade channel management
self.channel_tracker = TradeChannelTracker() self.channel_tracker = TradeChannelTracker()
self.channel_manager = TradeChannelManager(self.channel_tracker) self.channel_manager = TradeChannelManager(self.channel_tracker)
# Create the trade command group # 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.""" """Get the trade channel for a given trade ID."""
channel_data = self.channel_tracker.get_channel_by_trade_id(trade_id) channel_data = self.channel_tracker.get_channel_by_trade_id(trade_id)
if not channel_data: if not channel_data:
@ -55,7 +65,9 @@ class TradeCommands(commands.Cog):
return channel return channel
return None 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.""" """Check if the interaction is happening in the trade's dedicated channel."""
trade_channel = self._get_trade_channel(interaction.guild, trade_id) trade_channel = self._get_trade_channel(interaction.guild, trade_id)
if not trade_channel: if not trade_channel:
@ -68,7 +80,7 @@ class TradeCommands(commands.Cog):
trade_id: str, trade_id: str,
embed: discord.Embed, embed: discord.Embed,
view: Optional[discord.ui.View] = None, view: Optional[discord.ui.View] = None,
content: Optional[str] = None content: Optional[str] = None,
) -> bool: ) -> bool:
""" """
Post the trade embed to the trade channel. Post the trade embed to the trade channel.
@ -90,19 +102,12 @@ class TradeCommands(commands.Cog):
return False return False
@trade_group.command( @trade_group.command(
name="initiate", name="initiate", description="Start a new trade with another team"
description="Start a new trade with another team"
)
@app_commands.describe(
other_team="Team abbreviation to trade with"
) )
@app_commands.describe(other_team="Team abbreviation to trade with")
@app_commands.autocomplete(other_team=major_league_team_autocomplete) @app_commands.autocomplete(other_team=major_league_team_autocomplete)
@logged_command("/trade initiate") @logged_command("/trade initiate")
async def trade_initiate( async def trade_initiate(self, interaction: discord.Interaction, other_team: str):
self,
interaction: discord.Interaction,
other_team: str
):
"""Initiate a new trade with another team.""" """Initiate a new trade with another team."""
await interaction.response.defer(ephemeral=True) await interaction.response.defer(ephemeral=True)
@ -112,15 +117,16 @@ class TradeCommands(commands.Cog):
return return
# Get the other team # 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: if not other_team_obj:
return return
# Check if it's the same team # Check if it's the same team
if user_team.id == other_team_obj.id: if user_team.id == other_team_obj.id:
await interaction.followup.send( await interaction.followup.send(
"❌ You cannot initiate a trade with yourself.", "❌ You cannot initiate a trade with yourself.", ephemeral=True
ephemeral=True
) )
return return
@ -133,7 +139,7 @@ class TradeCommands(commands.Cog):
if not success: if not success:
await interaction.followup.send( await interaction.followup.send(
f"❌ Failed to add {other_team_obj.abbrev} to trade: {error_msg}", f"❌ Failed to add {other_team_obj.abbrev} to trade: {error_msg}",
ephemeral=True ephemeral=True,
) )
return return
@ -143,7 +149,7 @@ class TradeCommands(commands.Cog):
trade_id=trade_builder.trade_id, trade_id=trade_builder.trade_id,
team1=user_team, team1=user_team,
team2=other_team_obj, team2=other_team_obj,
creator_id=interaction.user.id creator_id=interaction.user.id,
) )
# Show trade interface # Show trade interface
@ -156,31 +162,26 @@ class TradeCommands(commands.Cog):
success_msg += f"\n📝 Discussion channel: {channel.mention}" success_msg += f"\n📝 Discussion channel: {channel.mention}"
else: else:
success_msg += f"\n⚠️ **Warning:** Failed to create discussion channel. Check bot permissions or contact an admin." 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( await interaction.followup.send(
content=success_msg, content=success_msg, embed=embed, view=view, ephemeral=True
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( @trade_group.command(
name="add-team", name="add-team",
description="Add another team to your current trade (for 3+ team trades)" 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.describe(other_team="Team abbreviation to add to the trade")
@app_commands.autocomplete(other_team=major_league_team_autocomplete) @app_commands.autocomplete(other_team=major_league_team_autocomplete)
@logged_command("/trade add-team") @logged_command("/trade add-team")
async def trade_add_team( async def trade_add_team(self, interaction: discord.Interaction, other_team: str):
self,
interaction: discord.Interaction,
other_team: str
):
"""Add a team to an existing trade.""" """Add a team to an existing trade."""
await interaction.response.defer(ephemeral=False) await interaction.response.defer(ephemeral=False)
@ -194,7 +195,7 @@ class TradeCommands(commands.Cog):
if not trade_builder: if not trade_builder:
await interaction.followup.send( await interaction.followup.send(
"❌ Your team is not part of an active trade. Use `/trade initiate` first.", "❌ Your team is not part of an active trade. Use `/trade initiate` first.",
ephemeral=True ephemeral=True,
) )
return return
@ -207,8 +208,7 @@ class TradeCommands(commands.Cog):
success, error_msg = await trade_builder.add_team(team_to_add) success, error_msg = await trade_builder.add_team(team_to_add)
if not success: if not success:
await interaction.followup.send( await interaction.followup.send(
f"❌ Failed to add {team_to_add.abbrev}: {error_msg}", f"❌ Failed to add {team_to_add.abbrev}: {error_msg}", ephemeral=True
ephemeral=True
) )
return return
@ -216,7 +216,7 @@ class TradeCommands(commands.Cog):
channel_updated = await self.channel_manager.add_team_to_channel( channel_updated = await self.channel_manager.add_team_to_channel(
guild=interaction.guild, guild=interaction.guild,
trade_id=trade_builder.trade_id, trade_id=trade_builder.trade_id,
new_team=team_to_add new_team=team_to_add,
) )
# Show updated trade interface # Show updated trade interface
@ -226,13 +226,12 @@ class TradeCommands(commands.Cog):
# Build success message # Build success message
success_msg = f"✅ **Added {team_to_add.abbrev} to the trade**" success_msg = f"✅ **Added {team_to_add.abbrev} to the trade**"
if channel_updated: 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( await interaction.followup.send(
content=success_msg, content=success_msg, embed=embed, view=view, ephemeral=True
embed=embed,
view=view,
ephemeral=True
) )
# If command was executed outside trade channel, post update to trade channel # 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, trade_id=trade_builder.trade_id,
embed=embed, embed=embed,
view=view, 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( @trade_group.command(name="add-player", description="Add a player to the trade")
name="add-player",
description="Add a player to the trade"
)
@app_commands.describe( @app_commands.describe(
player_name="Player name; begin typing for autocomplete", 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(player_name=player_autocomplete)
@app_commands.autocomplete(destination_team=team_autocomplete) @app_commands.autocomplete(destination_team=team_autocomplete)
@logged_command("/trade add-player") @logged_command("/trade add-player")
async def trade_add_player( async def trade_add_player(
self, self, interaction: discord.Interaction, player_name: str, destination_team: str
interaction: discord.Interaction,
player_name: str,
destination_team: str
): ):
"""Add a player move to the trade.""" """Add a player move to the trade."""
await interaction.response.defer(ephemeral=False) await interaction.response.defer(ephemeral=False)
@ -277,16 +272,17 @@ class TradeCommands(commands.Cog):
if not trade_builder: if not trade_builder:
await interaction.followup.send( await interaction.followup.send(
"❌ Your team is not part of an active trade. Use `/trade initiate` or ask another GM to add your team.", "❌ 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 return
# Find the player # 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: if not players:
await interaction.followup.send( await interaction.followup.send(
f"❌ Player '{player_name}' not found.", f"❌ Player '{player_name}' not found.", ephemeral=True
ephemeral=True
) )
return return
@ -300,15 +296,19 @@ class TradeCommands(commands.Cog):
player = players[0] player = players[0]
# Get destination team # 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: if not dest_team:
return return
# Determine source team and roster locations # Auto-detect source roster from player's actual team assignment
# For now, assume player comes from user's team and goes to ML of destination player_team = await team_service.get_team(player.team_id)
# The service will validate that the player is actually on the user's team organization if player_team:
from_roster = RosterType.MAJOR_LEAGUE # Default assumption from_roster = player_team.roster_type()
to_roster = RosterType.MAJOR_LEAGUE # Default destination else:
from_roster = RosterType.MAJOR_LEAGUE # Fallback
to_roster = dest_team.roster_type()
# Add the player move (service layer will validate) # Add the player move (service layer will validate)
success, error_msg = await trade_builder.add_player_move( success, error_msg = await trade_builder.add_player_move(
@ -316,26 +316,22 @@ class TradeCommands(commands.Cog):
from_team=user_team, from_team=user_team,
to_team=dest_team, to_team=dest_team,
from_roster=from_roster, from_roster=from_roster,
to_roster=to_roster to_roster=to_roster,
) )
if not success: if not success:
await interaction.followup.send( await interaction.followup.send(f"{error_msg}", ephemeral=True)
f"{error_msg}",
ephemeral=True
)
return return
# Show updated trade interface # Show updated trade interface
embed = await create_trade_embed(trade_builder) embed = await create_trade_embed(trade_builder)
view = TradeEmbedView(trade_builder, interaction.user.id) 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( await interaction.followup.send(
content=success_msg, content=success_msg, embed=embed, view=view, ephemeral=True
embed=embed,
view=view,
ephemeral=True
) )
# If command was executed outside trade channel, post update to trade channel # 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, trade_id=trade_builder.trade_id,
embed=embed, embed=embed,
view=view, 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( @trade_group.command(
name="supplementary", 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( @app_commands.describe(
player_name="Player name; begin typing for autocomplete", 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.autocomplete(player_name=player_autocomplete)
@app_commands.choices(destination=[ @app_commands.choices(
app_commands.Choice(name="Major League", value="ml"), destination=[
app_commands.Choice(name="Minor League", value="mil"), app_commands.Choice(name="Major League", value="ml"),
app_commands.Choice(name="Free Agency", value="fa") app_commands.Choice(name="Minor League", value="mil"),
]) app_commands.Choice(name="Free Agency", value="fa"),
]
)
@logged_command("/trade supplementary") @logged_command("/trade supplementary")
async def trade_supplementary( async def trade_supplementary(
self, self, interaction: discord.Interaction, player_name: str, destination: str
interaction: discord.Interaction,
player_name: str,
destination: str
): ):
"""Add a supplementary (internal organization) move for roster legality.""" """Add a supplementary (internal organization) move for roster legality."""
await interaction.response.defer(ephemeral=False) await interaction.response.defer(ephemeral=False)
@ -384,16 +381,17 @@ class TradeCommands(commands.Cog):
if not trade_builder: if not trade_builder:
await interaction.followup.send( await interaction.followup.send(
"❌ Your team is not part of an active trade. Use `/trade initiate` or ask another GM to add your team.", "❌ 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 return
# Find the player # 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: if not players:
await interaction.followup.send( await interaction.followup.send(
f"❌ Player '{player_name}' not found.", f"❌ Player '{player_name}' not found.", ephemeral=True
ephemeral=True
) )
return return
@ -403,45 +401,47 @@ class TradeCommands(commands.Cog):
destination_map = { destination_map = {
"ml": RosterType.MAJOR_LEAGUE, "ml": RosterType.MAJOR_LEAGUE,
"mil": RosterType.MINOR_LEAGUE, "mil": RosterType.MINOR_LEAGUE,
"fa": RosterType.FREE_AGENCY "fa": RosterType.FREE_AGENCY,
} }
to_roster = destination_map.get(destination.lower()) to_roster = destination_map.get(destination.lower())
if not to_roster: if not to_roster:
await interaction.followup.send( await interaction.followup.send(
f"❌ Invalid destination: {destination}", f"❌ Invalid destination: {destination}", ephemeral=True
ephemeral=True
) )
return return
# Determine current roster (default assumption) # Auto-detect source roster from player's actual team assignment
from_roster = RosterType.MINOR_LEAGUE if to_roster == RosterType.MAJOR_LEAGUE else RosterType.MAJOR_LEAGUE 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 # Add supplementary move
success, error_msg = await trade_builder.add_supplementary_move( success, error_msg = await trade_builder.add_supplementary_move(
team=user_team, team=user_team, player=player, from_roster=from_roster, to_roster=to_roster
player=player,
from_roster=from_roster,
to_roster=to_roster
) )
if not success: if not success:
await interaction.followup.send( await interaction.followup.send(
f"❌ Failed to add supplementary move: {error_msg}", f"❌ Failed to add supplementary move: {error_msg}", ephemeral=True
ephemeral=True
) )
return return
# Show updated trade interface # Show updated trade interface
embed = await create_trade_embed(trade_builder) embed = await create_trade_embed(trade_builder)
view = TradeEmbedView(trade_builder, interaction.user.id) 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( await interaction.followup.send(
content=success_msg, content=success_msg, embed=embed, view=view, ephemeral=True
embed=embed,
view=view,
ephemeral=True
) )
# If command was executed outside trade channel, post update to trade channel # 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, trade_id=trade_builder.trade_id,
embed=embed, embed=embed,
view=view, 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( @trade_group.command(name="view", description="View your current trade")
name="view",
description="View your current trade"
)
@logged_command("/trade view") @logged_command("/trade view")
async def trade_view(self, interaction: discord.Interaction): async def trade_view(self, interaction: discord.Interaction):
"""View the current trade.""" """View the current trade."""
@ -474,8 +473,7 @@ class TradeCommands(commands.Cog):
trade_builder = get_trade_builder_by_team(user_team.id) trade_builder = get_trade_builder_by_team(user_team.id)
if not trade_builder: if not trade_builder:
await interaction.followup.send( await interaction.followup.send(
"❌ Your team is not part of an active trade.", "❌ Your team is not part of an active trade.", ephemeral=True
ephemeral=True
) )
return return
@ -483,11 +481,7 @@ class TradeCommands(commands.Cog):
embed = await create_trade_embed(trade_builder) embed = await create_trade_embed(trade_builder)
view = TradeEmbedView(trade_builder, interaction.user.id) view = TradeEmbedView(trade_builder, interaction.user.id)
await interaction.followup.send( await interaction.followup.send(embed=embed, view=view, ephemeral=True)
embed=embed,
view=view,
ephemeral=True
)
# If command was executed outside trade channel, post update to trade channel # If command was executed outside trade channel, post update to trade channel
if not self._is_in_trade_channel(interaction, trade_builder.trade_id): if not self._is_in_trade_channel(interaction, trade_builder.trade_id):
@ -495,13 +489,10 @@ class TradeCommands(commands.Cog):
guild=interaction.guild, guild=interaction.guild,
trade_id=trade_builder.trade_id, trade_id=trade_builder.trade_id,
embed=embed, embed=embed,
view=view view=view,
) )
@trade_group.command( @trade_group.command(name="clear", description="Clear your current trade")
name="clear",
description="Clear your current trade"
)
@logged_command("/trade clear") @logged_command("/trade clear")
async def trade_clear(self, interaction: discord.Interaction): async def trade_clear(self, interaction: discord.Interaction):
"""Clear the current trade.""" """Clear the current trade."""
@ -516,8 +507,7 @@ class TradeCommands(commands.Cog):
trade_builder = get_trade_builder_by_team(user_team.id) trade_builder = get_trade_builder_by_team(user_team.id)
if not trade_builder: if not trade_builder:
await interaction.followup.send( await interaction.followup.send(
"❌ Your team is not part of an active trade.", "❌ Your team is not part of an active trade.", ephemeral=True
ephemeral=True
) )
return return
@ -525,19 +515,17 @@ class TradeCommands(commands.Cog):
# Delete associated trade channel if it exists # Delete associated trade channel if it exists
await self.channel_manager.delete_trade_channel( await self.channel_manager.delete_trade_channel(
guild=interaction.guild, guild=interaction.guild, trade_id=trade_id
trade_id=trade_id
) )
# Clear the trade builder using team-based function # Clear the trade builder using team-based function
clear_trade_builder_by_team(user_team.id) clear_trade_builder_by_team(user_team.id)
await interaction.followup.send( await interaction.followup.send(
"✅ The trade has been cleared.", "✅ The trade has been cleared.", ephemeral=True
ephemeral=True
) )
async def setup(bot): async def setup(bot):
"""Setup function for the cog.""" """Setup function for the cog."""
await bot.add_cog(TradeCommands(bot)) await bot.add_cog(TradeCommands(bot))