From e98a658fdea078a8eaa38a1d650452178c997639 Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Sun, 8 Mar 2026 11:21:51 -0500 Subject: [PATCH 1/2] fix: show actual validation errors in trade embed Quick Status Quick Status previously only showed "X errors found" with no details. Now lists each error and suggestion inline. Also stripped all emoji from embed titles, field names, values, buttons, and messages. Co-Authored-By: Claude Opus 4.6 --- views/trade_embed.py | 449 +++++++++++++++++++++---------------------- 1 file changed, 222 insertions(+), 227 deletions(-) diff --git a/views/trade_embed.py b/views/trade_embed.py index 507299f..acccfa7 100644 --- a/views/trade_embed.py +++ b/views/trade_embed.py @@ -3,6 +3,7 @@ Interactive Trade Embed Views Handles the Discord embed and button interfaces for the multi-team trade builder. """ + import discord from typing import Optional, List from datetime import datetime, timezone @@ -31,60 +32,56 @@ class TradeEmbedView(discord.ui.View): """Check if user has permission to interact with this view.""" if interaction.user.id != self.user_id: await interaction.response.send_message( - "āŒ You don't have permission to use this trade builder.", - ephemeral=True + "You don't have permission to use this trade builder.", + ephemeral=True, ) return False return True async def on_timeout(self) -> None: """Handle view timeout.""" - # Disable all buttons when timeout occurs for item in self.children: if isinstance(item, discord.ui.Button): item.disabled = True - @discord.ui.button(label="Remove Move", style=discord.ButtonStyle.red, emoji="āž–") - async def remove_move_button(self, interaction: discord.Interaction, button: discord.ui.Button): + @discord.ui.button(label="Remove Move", style=discord.ButtonStyle.red) + async def remove_move_button( + self, interaction: discord.Interaction, button: discord.ui.Button + ): """Handle remove move button click.""" if self.builder.is_empty: await interaction.response.send_message( - "āŒ No moves to remove. Add some moves first!", - ephemeral=True + "No moves to remove. Add some moves first!", ephemeral=True ) return - # Create select menu for move removal select_view = RemoveTradeMovesView(self.builder, self.user_id) embed = await create_trade_embed(self.builder) await interaction.response.edit_message(embed=embed, view=select_view) - @discord.ui.button(label="Validate Trade", style=discord.ButtonStyle.secondary, emoji="šŸ”") - async def validate_button(self, interaction: discord.Interaction, button: discord.ui.Button): + @discord.ui.button(label="Validate Trade", style=discord.ButtonStyle.secondary) + async def validate_button( + self, interaction: discord.Interaction, button: discord.ui.Button + ): """Handle validate trade button click.""" await interaction.response.defer(ephemeral=True) - # Perform detailed validation validation = await self.builder.validate_trade() - # Create validation report if validation.is_legal: - status_emoji = "āœ…" status_text = "**Trade is LEGAL**" color = EmbedColors.SUCCESS else: - status_emoji = "āŒ" status_text = "**Trade has ERRORS**" color = EmbedColors.ERROR embed = EmbedTemplate.create_base_embed( - title=f"{status_emoji} Trade Validation Report", + title="Trade Validation Report", description=status_text, - color=color + color=color, ) - # Add team-by-team validation for participant in self.builder.trade.participants: team_validation = validation.get_participant_validation(participant.team.id) if team_validation: @@ -98,72 +95,65 @@ class TradeEmbedView(discord.ui.View): team_status.append(team_validation.pre_existing_transactions_note) embed.add_field( - name=f"šŸŸļø {participant.team.abbrev} - {participant.team.sname}", + name=f"{participant.team.abbrev} - {participant.team.sname}", value="\n".join(team_status), - inline=False + inline=False, ) - # Add overall errors and suggestions if validation.all_errors: - error_text = "\n".join([f"ā€¢ {error}" for error in validation.all_errors]) - embed.add_field( - name="āŒ Errors", - value=error_text, - inline=False - ) + error_text = "\n".join([f"- {error}" for error in validation.all_errors]) + embed.add_field(name="Errors", value=error_text, inline=False) if validation.all_suggestions: - suggestion_text = "\n".join([f"šŸ’” {suggestion}" for suggestion in validation.all_suggestions]) - embed.add_field( - name="šŸ’” Suggestions", - value=suggestion_text, - inline=False + suggestion_text = "\n".join( + [f"- {suggestion}" for suggestion in validation.all_suggestions] ) + embed.add_field(name="Suggestions", value=suggestion_text, inline=False) await interaction.followup.send(embed=embed, ephemeral=True) - @discord.ui.button(label="Submit Trade", style=discord.ButtonStyle.primary, emoji="šŸ“¤") - async def submit_button(self, interaction: discord.Interaction, button: discord.ui.Button): + @discord.ui.button(label="Submit Trade", style=discord.ButtonStyle.primary) + async def submit_button( + self, interaction: discord.Interaction, button: discord.ui.Button + ): """Handle submit trade button click.""" if self.builder.is_empty: await interaction.response.send_message( - "āŒ Cannot submit empty trade. Add some moves first!", - ephemeral=True + "Cannot submit empty trade. Add some moves first!", ephemeral=True ) return - # Validate before submission validation = await self.builder.validate_trade() if not validation.is_legal: - error_msg = "āŒ **Cannot submit illegal trade:**\n" - error_msg += "\n".join([f"ā€¢ {error}" for error in validation.all_errors]) + error_msg = "**Cannot submit illegal trade:**\n" + error_msg += "\n".join([f"- {error}" for error in validation.all_errors]) if validation.all_suggestions: error_msg += "\n\n**Suggestions:**\n" - error_msg += "\n".join([f"šŸ’” {suggestion}" for suggestion in validation.all_suggestions]) + error_msg += "\n".join( + [f"- {suggestion}" for suggestion in validation.all_suggestions] + ) await interaction.response.send_message(error_msg, ephemeral=True) return - # Show confirmation modal modal = SubmitTradeConfirmationModal(self.builder) await interaction.response.send_modal(modal) - @discord.ui.button(label="Cancel Trade", style=discord.ButtonStyle.secondary, emoji="āŒ") - async def cancel_button(self, interaction: discord.Interaction, button: discord.ui.Button): + @discord.ui.button(label="Cancel Trade", style=discord.ButtonStyle.secondary) + async def cancel_button( + self, interaction: discord.Interaction, button: discord.ui.Button + ): """Handle cancel trade button click.""" self.builder.clear_trade() embed = await create_trade_embed(self.builder) - # Disable all buttons after cancellation for item in self.children: if isinstance(item, discord.ui.Button): item.disabled = True await interaction.response.edit_message( - content="āŒ **Trade cancelled and cleared.**", - embed=embed, - view=self + content="**Trade cancelled and cleared.**", embed=embed, view=self ) self.stop() @@ -176,12 +166,12 @@ class RemoveTradeMovesView(discord.ui.View): self.builder = builder self.user_id = user_id - # Create select menu with current moves if not builder.is_empty: self.add_item(RemoveTradeMovesSelect(builder)) - # Add back button - back_button = discord.ui.Button(label="Back", style=discord.ButtonStyle.secondary, emoji="ā¬…ļø") + back_button = discord.ui.Button( + label="Back", style=discord.ButtonStyle.secondary + ) back_button.callback = self.back_callback self.add_item(back_button) @@ -202,35 +192,36 @@ class RemoveTradeMovesSelect(discord.ui.Select): def __init__(self, builder: TradeBuilder): self.builder = builder - # Create options from all moves (cross-team and supplementary) options = [] move_count = 0 - # Add cross-team moves - for move in builder.trade.cross_team_moves[:20]: # Limit to avoid Discord's 25 option limit - options.append(discord.SelectOption( - label=f"{move.player.name}", - description=move.description[:100], # Discord description limit - value=str(move.player.id), - emoji="šŸ”„" - )) + for move in builder.trade.cross_team_moves[ + :20 + ]: # Limit to avoid Discord's 25 option limit + options.append( + discord.SelectOption( + label=f"{move.player.name}", + description=move.description[:100], + value=str(move.player.id), + ) + ) move_count += 1 - # Add supplementary moves if there's room remaining_slots = 25 - move_count for move in builder.trade.supplementary_moves[:remaining_slots]: - options.append(discord.SelectOption( - label=f"{move.player.name}", - description=move.description[:100], - value=str(move.player.id), - emoji="āš™ļø" - )) + options.append( + discord.SelectOption( + label=f"{move.player.name}", + description=move.description[:100], + value=str(move.player.id), + ) + ) super().__init__( placeholder="Select a move to remove...", min_values=1, max_values=1, - options=options + options=options, ) async def callback(self, interaction: discord.Interaction): @@ -241,27 +232,25 @@ class RemoveTradeMovesSelect(discord.ui.Select): if success: await interaction.response.send_message( - f"āœ… Removed move for player ID {player_id}", - ephemeral=True + f"Removed move for player ID {player_id}", ephemeral=True ) - # Update the embed main_view = TradeEmbedView(self.builder, interaction.user.id) embed = await create_trade_embed(self.builder) - # Edit the original message await interaction.edit_original_response(embed=embed, view=main_view) else: await interaction.response.send_message( - f"āŒ Could not remove move: {error_msg}", - ephemeral=True + f"Could not remove move: {error_msg}", ephemeral=True ) class SubmitTradeConfirmationModal(discord.ui.Modal): """Modal for confirming trade submission - posts acceptance request to trade channel.""" - def __init__(self, builder: TradeBuilder, trade_channel: Optional[discord.TextChannel] = None): + def __init__( + self, builder: TradeBuilder, trade_channel: Optional[discord.TextChannel] = None + ): super().__init__(title="Confirm Trade Submission") self.builder = builder self.trade_channel = trade_channel @@ -270,7 +259,7 @@ class SubmitTradeConfirmationModal(discord.ui.Modal): label="Type 'CONFIRM' to submit for approval", placeholder="CONFIRM", required=True, - max_length=7 + max_length=7, ) self.add_item(self.confirmation) @@ -279,56 +268,52 @@ class SubmitTradeConfirmationModal(discord.ui.Modal): """Handle confirmation submission - posts acceptance view to trade channel.""" if self.confirmation.value.upper() != "CONFIRM": await interaction.response.send_message( - "āŒ Trade not submitted. You must type 'CONFIRM' exactly.", - ephemeral=True + "Trade not submitted. You must type 'CONFIRM' exactly.", + ephemeral=True, ) return await interaction.response.defer(ephemeral=True) try: - # Update trade status to PROPOSED from models.trade import TradeStatus + self.builder.trade.status = TradeStatus.PROPOSED - # Create acceptance embed and view acceptance_embed = await create_trade_acceptance_embed(self.builder) acceptance_view = TradeAcceptanceView(self.builder) - # Find the trade channel to post to channel = self.trade_channel if not channel: - # Try to find trade channel by name pattern - trade_channel_name = f"trade-{'-'.join(t.abbrev.lower() for t in self.builder.participating_teams)}" for ch in interaction.guild.text_channels: # type: ignore - if ch.name.startswith("trade-") and self.builder.trade_id[:4] in ch.name: + if ( + ch.name.startswith("trade-") + and self.builder.trade_id[:4] in ch.name + ): channel = ch break if channel: - # Post acceptance request to trade channel await channel.send( - content="šŸ“‹ **Trade submitted for approval!** All teams must accept to complete the trade.", + content="**Trade submitted for approval.** All teams must accept to complete the trade.", embed=acceptance_embed, - view=acceptance_view + view=acceptance_view, ) await interaction.followup.send( - f"āœ… Trade submitted for approval!\n\nThe acceptance request has been posted to {channel.mention}.\n" + f"Trade submitted for approval.\n\nThe acceptance request has been posted to {channel.mention}.\n" f"All participating teams must click **Accept Trade** to finalize.", - ephemeral=True + ephemeral=True, ) else: - # No trade channel found, post in current channel await interaction.followup.send( - content="šŸ“‹ **Trade submitted for approval!** All teams must accept to complete the trade.", + content="**Trade submitted for approval.** All teams must accept to complete the trade.", embed=acceptance_embed, - view=acceptance_view + view=acceptance_view, ) except Exception as e: await interaction.followup.send( - f"āŒ Error submitting trade: {str(e)}", - ephemeral=True + f"Error submitting trade: {str(e)}", ephemeral=True ) @@ -343,8 +328,11 @@ class TradeAcceptanceView(discord.ui.View): """Get the team owned by the interacting user.""" from services.team_service import team_service from config import get_config + config = get_config() - return await team_service.get_team_by_owner(interaction.user.id, config.sba_season) + return await team_service.get_team_by_owner( + interaction.user.id, config.sba_season + ) async def interaction_check(self, interaction: discord.Interaction) -> bool: """Check if user is a GM of a participating team.""" @@ -352,17 +340,14 @@ class TradeAcceptanceView(discord.ui.View): if not user_team: await interaction.response.send_message( - "āŒ You don't own a team in the league.", - ephemeral=True + "You don't own a team in the league.", ephemeral=True ) return False - # Check if their team (or organization) is participating participant = self.builder.trade.get_participant_by_organization(user_team) if not participant: await interaction.response.send_message( - "āŒ Your team is not part of this trade.", - ephemeral=True + "Your team is not part of this trade.", ephemeral=True ) return False @@ -374,47 +359,45 @@ class TradeAcceptanceView(discord.ui.View): if isinstance(item, discord.ui.Button): item.disabled = True - @discord.ui.button(label="Accept Trade", style=discord.ButtonStyle.success, emoji="āœ…") - async def accept_button(self, interaction: discord.Interaction, button: discord.ui.Button): + @discord.ui.button(label="Accept Trade", style=discord.ButtonStyle.success) + async def accept_button( + self, interaction: discord.Interaction, button: discord.ui.Button + ): """Handle accept button click.""" user_team = await self._get_user_team(interaction) if not user_team: return - # Find the participating team (could be org affiliate) participant = self.builder.trade.get_participant_by_organization(user_team) if not participant: return team_id = participant.team.id - # Check if already accepted if self.builder.has_team_accepted(team_id): await interaction.response.send_message( - f"āœ… {participant.team.abbrev} has already accepted this trade.", - ephemeral=True + f"{participant.team.abbrev} has already accepted this trade.", + ephemeral=True, ) return - # Record acceptance all_accepted = self.builder.accept_trade(team_id) if all_accepted: - # All teams accepted - finalize the trade await self._finalize_trade(interaction) else: - # Update embed to show new acceptance status embed = await create_trade_acceptance_embed(self.builder) await interaction.response.edit_message(embed=embed, view=self) - # Send confirmation to channel await interaction.followup.send( - f"āœ… **{participant.team.abbrev}** has accepted the trade! " + f"**{participant.team.abbrev}** has accepted the trade. " f"({len(self.builder.accepted_teams)}/{self.builder.team_count} teams)" ) - @discord.ui.button(label="Reject Trade", style=discord.ButtonStyle.danger, emoji="āŒ") - async def reject_button(self, interaction: discord.Interaction, button: discord.ui.Button): + @discord.ui.button(label="Reject Trade", style=discord.ButtonStyle.danger) + async def reject_button( + self, interaction: discord.Interaction, button: discord.ui.Button + ): """Handle reject button click - moves trade back to DRAFT.""" user_team = await self._get_user_team(interaction) if not user_team: @@ -424,20 +407,16 @@ class TradeAcceptanceView(discord.ui.View): if not participant: return - # Reject the trade self.builder.reject_trade() - # Disable buttons self.accept_button.disabled = True self.reject_button.disabled = True - # Update embed to show rejection embed = await create_trade_rejection_embed(self.builder, participant.team) await interaction.response.edit_message(embed=embed, view=self) - # Notify the channel await interaction.followup.send( - f"āŒ **{participant.team.abbrev}** has rejected the trade.\n\n" + f"**{participant.team.abbrev}** has rejected the trade.\n\n" f"The trade has been moved back to **DRAFT** status. " f"Teams can continue negotiating using `/trade` commands." ) @@ -459,41 +438,52 @@ class TradeAcceptanceView(discord.ui.View): config = get_config() - # Get next week for transactions current = await league_service.get_current_state() next_week = current.week + 1 if current else 1 - # Create FA team for reference fa_team = Team( id=config.free_agent_team_id, abbrev="FA", sname="Free Agents", lname="Free Agency", - season=self.builder.trade.season + season=self.builder.trade.season, ) # type: ignore - # Create transactions from all moves transactions: List[Transaction] = [] move_id = f"Trade-{self.builder.trade_id}-{int(datetime.now(timezone.utc).timestamp())}" - # Process cross-team moves for move in self.builder.trade.cross_team_moves: - # Get actual team affiliates for from/to based on roster type if move.from_roster == RosterType.MAJOR_LEAGUE: old_team = move.source_team elif move.from_roster == RosterType.MINOR_LEAGUE: - old_team = await move.source_team.minor_league_affiliate() if move.source_team else None + old_team = ( + await move.source_team.minor_league_affiliate() + if move.source_team + else None + ) elif move.from_roster == RosterType.INJURED_LIST: - old_team = await move.source_team.injured_list_affiliate() if move.source_team else None + old_team = ( + await move.source_team.injured_list_affiliate() + if move.source_team + else None + ) else: old_team = move.source_team if move.to_roster == RosterType.MAJOR_LEAGUE: new_team = move.destination_team elif move.to_roster == RosterType.MINOR_LEAGUE: - new_team = await move.destination_team.minor_league_affiliate() if move.destination_team else None + new_team = ( + await move.destination_team.minor_league_affiliate() + if move.destination_team + else None + ) elif move.to_roster == RosterType.INJURED_LIST: - new_team = await move.destination_team.injured_list_affiliate() if move.destination_team else None + new_team = ( + await move.destination_team.injured_list_affiliate() + if move.destination_team + else None + ) else: new_team = move.destination_team @@ -507,18 +497,25 @@ class TradeAcceptanceView(discord.ui.View): oldteam=old_team, newteam=new_team, cancelled=False, - frozen=False # Trades are NOT frozen - immediately effective + frozen=False, ) transactions.append(transaction) - # Process supplementary moves for move in self.builder.trade.supplementary_moves: if move.from_roster == RosterType.MAJOR_LEAGUE: old_team = move.source_team elif move.from_roster == RosterType.MINOR_LEAGUE: - old_team = await move.source_team.minor_league_affiliate() if move.source_team else None + old_team = ( + await move.source_team.minor_league_affiliate() + if move.source_team + else None + ) elif move.from_roster == RosterType.INJURED_LIST: - old_team = await move.source_team.injured_list_affiliate() if move.source_team else None + old_team = ( + await move.source_team.injured_list_affiliate() + if move.source_team + else None + ) elif move.from_roster == RosterType.FREE_AGENCY: old_team = fa_team else: @@ -527,9 +524,17 @@ class TradeAcceptanceView(discord.ui.View): if move.to_roster == RosterType.MAJOR_LEAGUE: new_team = move.destination_team elif move.to_roster == RosterType.MINOR_LEAGUE: - new_team = await move.destination_team.minor_league_affiliate() if move.destination_team else None + new_team = ( + await move.destination_team.minor_league_affiliate() + if move.destination_team + else None + ) elif move.to_roster == RosterType.INJURED_LIST: - new_team = await move.destination_team.injured_list_affiliate() if move.destination_team else None + new_team = ( + await move.destination_team.injured_list_affiliate() + if move.destination_team + else None + ) elif move.to_roster == RosterType.FREE_AGENCY: new_team = fa_team else: @@ -545,45 +550,42 @@ class TradeAcceptanceView(discord.ui.View): oldteam=old_team, newteam=new_team, cancelled=False, - frozen=False # Trades are NOT frozen - immediately effective + frozen=False, ) transactions.append(transaction) - # POST transactions to database if transactions: - created_transactions = await transaction_service.create_transaction_batch(transactions) + created_transactions = ( + await transaction_service.create_transaction_batch(transactions) + ) else: created_transactions = [] - # Post to #transaction-log channel if created_transactions and interaction.client: await post_trade_to_log( bot=interaction.client, builder=self.builder, transactions=created_transactions, - effective_week=next_week + effective_week=next_week, ) - # Update trade status self.builder.trade.status = TradeStatus.ACCEPTED - # Disable buttons self.accept_button.disabled = True self.reject_button.disabled = True - # Update embed to show completion - embed = await create_trade_complete_embed(self.builder, len(created_transactions), next_week) + embed = await create_trade_complete_embed( + self.builder, len(created_transactions), next_week + ) await interaction.edit_original_response(embed=embed, view=self) - # Send completion message await interaction.followup.send( - f"šŸŽ‰ **Trade Complete!**\n\n" + f"**Trade Complete!**\n\n" f"All {self.builder.team_count} teams have accepted the trade.\n" f"**{len(created_transactions)} transactions** have been created for **Week {next_week}**.\n\n" f"Trade ID: `{self.builder.trade_id}`" ) - # Clear the trade builder for team in self.builder.participating_teams: clear_trade_builder_by_team(team.id) @@ -591,81 +593,79 @@ class TradeAcceptanceView(discord.ui.View): except Exception as e: await interaction.followup.send( - f"āŒ Error finalizing trade: {str(e)}", - ephemeral=True + f"Error finalizing trade: {str(e)}", ephemeral=True ) async def create_trade_acceptance_embed(builder: TradeBuilder) -> discord.Embed: """Create embed showing trade details and acceptance status.""" embed = EmbedTemplate.create_base_embed( - title=f"šŸ“‹ Trade Pending Acceptance - {builder.trade.get_trade_summary()}", + title=f"Trade Pending Acceptance - {builder.trade.get_trade_summary()}", description="All participating teams must accept to complete the trade.", - color=EmbedColors.WARNING + color=EmbedColors.WARNING, ) - # Show participating teams - team_list = [f"ā€¢ {team.abbrev} - {team.sname}" for team in builder.participating_teams] + team_list = [ + f"- {team.abbrev} - {team.sname}" for team in builder.participating_teams + ] embed.add_field( - name=f"šŸŸļø Participating Teams ({builder.team_count})", + name=f"Participating Teams ({builder.team_count})", value="\n".join(team_list), - inline=False + inline=False, ) - # Show cross-team moves if builder.trade.cross_team_moves: moves_text = "" for move in builder.trade.cross_team_moves[:10]: - moves_text += f"ā€¢ {move.description}\n" + moves_text += f"- {move.description}\n" if len(builder.trade.cross_team_moves) > 10: moves_text += f"... and {len(builder.trade.cross_team_moves) - 10} more" embed.add_field( - name=f"šŸ”„ Player Exchanges ({len(builder.trade.cross_team_moves)})", + name=f"Player Exchanges ({len(builder.trade.cross_team_moves)})", value=moves_text, - inline=False + inline=False, ) - # Show supplementary moves if any if builder.trade.supplementary_moves: supp_text = "" for move in builder.trade.supplementary_moves[:5]: - supp_text += f"ā€¢ {move.description}\n" + supp_text += f"- {move.description}\n" if len(builder.trade.supplementary_moves) > 5: supp_text += f"... and {len(builder.trade.supplementary_moves) - 5} more" embed.add_field( - name=f"āš™ļø Supplementary Moves ({len(builder.trade.supplementary_moves)})", + name=f"Supplementary Moves ({len(builder.trade.supplementary_moves)})", value=supp_text, - inline=False + inline=False, ) - # Show acceptance status status_lines = [] for team in builder.participating_teams: if team.id in builder.accepted_teams: - status_lines.append(f"āœ… **{team.abbrev}** - Accepted") + status_lines.append(f"**{team.abbrev}** - Accepted") else: - status_lines.append(f"ā³ **{team.abbrev}** - Pending") + status_lines.append(f"**{team.abbrev}** - Pending") embed.add_field( - name="šŸ“Š Acceptance Status", - value="\n".join(status_lines), - inline=False + name="Acceptance Status", value="\n".join(status_lines), inline=False ) - # Add footer - embed.set_footer(text=f"Trade ID: {builder.trade_id} ā€¢ {len(builder.accepted_teams)}/{builder.team_count} teams accepted") + embed.set_footer( + text=f"Trade ID: {builder.trade_id} | {len(builder.accepted_teams)}/{builder.team_count} teams accepted" + ) return embed -async def create_trade_rejection_embed(builder: TradeBuilder, rejecting_team: Team) -> discord.Embed: +async def create_trade_rejection_embed( + builder: TradeBuilder, rejecting_team: Team +) -> discord.Embed: """Create embed showing trade was rejected.""" embed = EmbedTemplate.create_base_embed( - title=f"āŒ Trade Rejected - {builder.trade.get_trade_summary()}", + title=f"Trade Rejected - {builder.trade.get_trade_summary()}", description=f"**{rejecting_team.abbrev}** has rejected the trade.\n\n" - f"The trade has been moved back to **DRAFT** status.\n" - f"Teams can continue negotiating using `/trade` commands.", - color=EmbedColors.ERROR + f"The trade has been moved back to **DRAFT** status.\n" + f"Teams can continue negotiating using `/trade` commands.", + color=EmbedColors.ERROR, ) embed.set_footer(text=f"Trade ID: {builder.trade_id}") @@ -673,37 +673,33 @@ async def create_trade_rejection_embed(builder: TradeBuilder, rejecting_team: Te return embed -async def create_trade_complete_embed(builder: TradeBuilder, transaction_count: int, effective_week: int) -> discord.Embed: +async def create_trade_complete_embed( + builder: TradeBuilder, transaction_count: int, effective_week: int +) -> discord.Embed: """Create embed showing trade was completed.""" embed = EmbedTemplate.create_base_embed( - title=f"šŸŽ‰ Trade Complete! - {builder.trade.get_trade_summary()}", - description=f"All {builder.team_count} teams have accepted the trade!\n\n" - f"**{transaction_count} transactions** created for **Week {effective_week}**.", - color=EmbedColors.SUCCESS + title=f"Trade Complete - {builder.trade.get_trade_summary()}", + description=f"All {builder.team_count} teams have accepted the trade.\n\n" + f"**{transaction_count} transactions** created for **Week {effective_week}**.", + color=EmbedColors.SUCCESS, ) - # Show final acceptance status (all green) - status_lines = [f"āœ… **{team.abbrev}** - Accepted" for team in builder.participating_teams] - embed.add_field( - name="šŸ“Š Final Status", - value="\n".join(status_lines), - inline=False - ) + status_lines = [ + f"**{team.abbrev}** - Accepted" for team in builder.participating_teams + ] + embed.add_field(name="Final Status", value="\n".join(status_lines), inline=False) - # Show cross-team moves if builder.trade.cross_team_moves: moves_text = "" for move in builder.trade.cross_team_moves[:8]: - moves_text += f"ā€¢ {move.description}\n" + moves_text += f"- {move.description}\n" if len(builder.trade.cross_team_moves) > 8: moves_text += f"... and {len(builder.trade.cross_team_moves) - 8} more" - embed.add_field( - name=f"šŸ”„ Player Exchanges", - value=moves_text, - inline=False - ) + embed.add_field(name="Player Exchanges", value=moves_text, inline=False) - embed.set_footer(text=f"Trade ID: {builder.trade_id} ā€¢ Effective: Week {effective_week}") + embed.set_footer( + text=f"Trade ID: {builder.trade_id} | Effective: Week {effective_week}" + ) return embed @@ -718,7 +714,6 @@ async def create_trade_embed(builder: TradeBuilder) -> discord.Embed: Returns: Discord embed with current trade state """ - # Determine embed color based on trade status if builder.is_empty: color = EmbedColors.SECONDARY else: @@ -726,79 +721,79 @@ async def create_trade_embed(builder: TradeBuilder) -> discord.Embed: color = EmbedColors.SUCCESS if validation.is_legal else EmbedColors.WARNING embed = EmbedTemplate.create_base_embed( - title=f"šŸ“‹ Trade Builder - {builder.trade.get_trade_summary()}", - description=f"Build your multi-team trade", - color=color + title=f"Trade Builder - {builder.trade.get_trade_summary()}", + description="Build your multi-team trade", + color=color, ) - # Add participating teams section - team_list = [f"ā€¢ {team.abbrev} - {team.sname}" for team in builder.participating_teams] + team_list = [ + f"- {team.abbrev} - {team.sname}" for team in builder.participating_teams + ] embed.add_field( - name=f"šŸŸļø Participating Teams ({builder.team_count})", + name=f"Participating Teams ({builder.team_count})", value="\n".join(team_list) if team_list else "*No teams yet*", - inline=False + inline=False, ) - # Add current moves section if builder.is_empty: embed.add_field( name="Current Moves", value="*No moves yet. Use the `/trade` commands to build your trade.*", - inline=False + inline=False, ) else: - # Show cross-team moves if builder.trade.cross_team_moves: moves_text = "" - for i, move in enumerate(builder.trade.cross_team_moves[:8], 1): # Limit display + for i, move in enumerate(builder.trade.cross_team_moves[:8], 1): moves_text += f"{i}. {move.description}\n" if len(builder.trade.cross_team_moves) > 8: moves_text += f"... and {len(builder.trade.cross_team_moves) - 8} more" embed.add_field( - name=f"šŸ”„ Player Exchanges ({len(builder.trade.cross_team_moves)})", + name=f"Player Exchanges ({len(builder.trade.cross_team_moves)})", value=moves_text, - inline=False + inline=False, ) - # Show supplementary moves if builder.trade.supplementary_moves: supp_text = "" - for i, move in enumerate(builder.trade.supplementary_moves[:5], 1): # Limit display + for i, move in enumerate(builder.trade.supplementary_moves[:5], 1): supp_text += f"{i}. {move.description}\n" if len(builder.trade.supplementary_moves) > 5: - supp_text += f"... and {len(builder.trade.supplementary_moves) - 5} more" + supp_text += ( + f"... and {len(builder.trade.supplementary_moves) - 5} more" + ) embed.add_field( - name=f"āš™ļø Supplementary Moves ({len(builder.trade.supplementary_moves)})", + name=f"Supplementary Moves ({len(builder.trade.supplementary_moves)})", value=supp_text, - inline=False + inline=False, ) - # Add quick validation summary validation = await builder.validate_trade() if validation.is_legal: - status_text = "āœ… Trade appears legal" + status_text = "Trade appears legal" else: error_count = len(validation.all_errors) - status_text = f"āŒ {error_count} error{'s' if error_count != 1 else ''} found" + status_text = f"{error_count} error{'s' if error_count != 1 else ''} found\n" + status_text += "\n".join(f"- {error}" for error in validation.all_errors) + if validation.all_suggestions: + status_text += "\n" + "\n".join( + f"- {s}" for s in validation.all_suggestions + ) + + embed.add_field(name="Quick Status", value=status_text, inline=False) embed.add_field( - name="šŸ” Quick Status", - value=status_text, - inline=False + name="Build Your Trade", + value="- `/trade add-player` - Add player exchanges\n- `/trade supplementary` - Add internal moves\n- `/trade add-team` - Add more teams", + inline=False, ) - # Add instructions for adding more moves - embed.add_field( - name="āž• Build Your Trade", - value="ā€¢ `/trade add-player` - Add player exchanges\nā€¢ `/trade supplementary` - Add internal moves\nā€¢ `/trade add-team` - Add more teams", - inline=False + embed.set_footer( + text=f"Trade ID: {builder.trade_id} | Created: {datetime.fromisoformat(builder.trade.created_at).strftime('%H:%M:%S')}" ) - # Add footer with trade ID and timestamp - embed.set_footer(text=f"Trade ID: {builder.trade_id} ā€¢ Created: {datetime.fromisoformat(builder.trade.created_at).strftime('%H:%M:%S')}") - - return embed \ No newline at end of file + return embed From 58fe9f22deda0341151d6b886dceb8a63795e1b5 Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Sun, 8 Mar 2026 11:25:31 -0500 Subject: [PATCH 2/2] fix: trade validation now checks against next week's projected roster validate_trade() was passing next_week=None to each team's validate_transaction(), which skipped load_existing_transactions() entirely. Trades were validated against the current roster only, ignoring pending /dropadd transactions for next week. Now auto-fetches current week from league_service and passes next_week=current_week+1, matching /dropadd validation behavior. Co-Authored-By: Claude Opus 4.6 --- services/trade_builder.py | 138 ++++++++++++++++++++++++++++---------- 1 file changed, 102 insertions(+), 36 deletions(-) diff --git a/services/trade_builder.py b/services/trade_builder.py index 879959f..8ab1d5d 100644 --- a/services/trade_builder.py +++ b/services/trade_builder.py @@ -3,6 +3,7 @@ Trade Builder Service Extends the TransactionBuilder to support multi-team trades and player exchanges. """ + import logging from typing import Dict, List, Optional, Set from datetime import datetime, timezone @@ -12,10 +13,14 @@ from config import get_config from models.trade import Trade, TradeMove, TradeStatus from models.team import Team, RosterType from models.player import Player -from services.transaction_builder import TransactionBuilder, RosterValidationResult, TransactionMove +from services.transaction_builder import ( + TransactionBuilder, + RosterValidationResult, + TransactionMove, +) from services.team_service import team_service -logger = logging.getLogger(f'{__name__}.TradeBuilder') +logger = logging.getLogger(f"{__name__}.TradeBuilder") class TradeValidationResult: @@ -52,7 +57,9 @@ class TradeValidationResult: suggestions.extend(validation.suggestions) return suggestions - def get_participant_validation(self, team_id: int) -> Optional[RosterValidationResult]: + def get_participant_validation( + self, team_id: int + ) -> Optional[RosterValidationResult]: """Get validation result for a specific team.""" return self.participant_validations.get(team_id) @@ -64,7 +71,12 @@ class TradeBuilder: Extends the functionality of TransactionBuilder to support trades between teams. """ - def __init__(self, initiated_by: int, initiating_team: Team, season: int = get_config().sba_season): + def __init__( + self, + initiated_by: int, + initiating_team: Team, + season: int = get_config().sba_season, + ): """ Initialize trade builder. @@ -79,7 +91,7 @@ class TradeBuilder: status=TradeStatus.DRAFT, initiated_by=initiated_by, created_at=datetime.now(timezone.utc).isoformat(), - season=season + season=season, ) # Add the initiating team as first participant @@ -91,7 +103,9 @@ class TradeBuilder: # Track which teams have accepted the trade (team_id -> True) self.accepted_teams: Set[int] = set() - logger.info(f"TradeBuilder initialized: {self.trade.trade_id} by user {initiated_by} for {initiating_team.abbrev}") + logger.info( + f"TradeBuilder initialized: {self.trade.trade_id} by user {initiated_by} for {initiating_team.abbrev}" + ) @property def trade_id(self) -> str: @@ -127,7 +141,11 @@ class TradeBuilder: @property def pending_teams(self) -> List[Team]: """Get list of teams that haven't accepted yet.""" - return [team for team in self.participating_teams if team.id not in self.accepted_teams] + return [ + team + for team in self.participating_teams + if team.id not in self.accepted_teams + ] def accept_trade(self, team_id: int) -> bool: """ @@ -140,7 +158,9 @@ class TradeBuilder: True if all teams have now accepted, False otherwise """ self.accepted_teams.add(team_id) - logger.info(f"Team {team_id} accepted trade {self.trade_id}. Accepted: {len(self.accepted_teams)}/{self.team_count}") + logger.info( + f"Team {team_id} accepted trade {self.trade_id}. Accepted: {len(self.accepted_teams)}/{self.team_count}" + ) return self.all_teams_accepted def reject_trade(self) -> None: @@ -160,7 +180,9 @@ class TradeBuilder: Returns: Dict mapping team_id to acceptance status (True/False) """ - return {team.id: team.id in self.accepted_teams for team in self.participating_teams} + return { + team.id: team.id in self.accepted_teams for team in self.participating_teams + } def has_team_accepted(self, team_id: int) -> bool: """Check if a specific team has accepted.""" @@ -184,7 +206,9 @@ class TradeBuilder: participant = self.trade.add_participant(team) # Create transaction builder for this team - self._team_builders[team.id] = TransactionBuilder(team, self.trade.initiated_by, self.trade.season) + self._team_builders[team.id] = TransactionBuilder( + team, self.trade.initiated_by, self.trade.season + ) # Register team in secondary index for multi-GM access trade_key = f"{self.trade.initiated_by}:trade" @@ -209,7 +233,10 @@ class TradeBuilder: # Check if team has moves - prevent removal if they do if participant.all_moves: - return False, f"{participant.team.abbrev} has moves in this trade and cannot be removed" + return ( + False, + f"{participant.team.abbrev} has moves in this trade and cannot be removed", + ) # Remove team removed = self.trade.remove_participant(team_id) @@ -229,7 +256,7 @@ class TradeBuilder: from_team: Team, to_team: Team, from_roster: RosterType, - to_roster: RosterType + to_roster: RosterType, ) -> tuple[bool, str]: """ Add a player move to the trade. @@ -246,7 +273,10 @@ class TradeBuilder: """ # Validate player is not from Free Agency if player.team_id == get_config().free_agent_team_id: - return False, f"Cannot add {player.name} from Free Agency. Players must be traded from teams within the organizations involved in the trade." + return ( + False, + f"Cannot add {player.name} from Free Agency. Players must be traded from teams within the organizations involved in the trade.", + ) # Validate player has a valid team assignment if not player.team_id: @@ -259,7 +289,10 @@ class TradeBuilder: # Check if player's team is in the same organization as from_team if not player_team.is_same_organization(from_team): - return False, f"{player.name} is on {player_team.abbrev}, they are not eligible to be added to the trade." + return ( + False, + f"{player.name} is on {player_team.abbrev}, they are not eligible to be added to the trade.", + ) # Ensure both teams are participating (check by organization for ML authority) from_participant = self.trade.get_participant_by_organization(from_team) @@ -274,7 +307,10 @@ class TradeBuilder: for participant in self.trade.participants: for existing_move in participant.all_moves: if existing_move.player.id == player.id: - return False, f"{player.name} is already involved in a move in this trade" + return ( + False, + f"{player.name} is already involved in a move in this trade", + ) # Create trade move trade_move = TradeMove( @@ -284,7 +320,7 @@ class TradeBuilder: from_team=from_team, to_team=to_team, source_team=from_team, - destination_team=to_team + destination_team=to_team, ) # Add to giving team's moves @@ -303,7 +339,7 @@ class TradeBuilder: from_roster=from_roster, to_roster=RosterType.FREE_AGENCY, # Conceptually leaving the org from_team=from_team, - to_team=None + to_team=None, ) # Move for receiving team (player joining) @@ -312,19 +348,23 @@ class TradeBuilder: from_roster=RosterType.FREE_AGENCY, # Conceptually joining from outside to_roster=to_roster, from_team=None, - to_team=to_team + to_team=to_team, ) # Add moves to respective builders # Skip pending transaction check for trades - they have their own validation workflow - from_success, from_error = await from_builder.add_move(from_move, check_pending_transactions=False) + from_success, from_error = await from_builder.add_move( + from_move, check_pending_transactions=False + ) if not from_success: # Remove from trade if builder failed from_participant.moves_giving.remove(trade_move) to_participant.moves_receiving.remove(trade_move) return False, f"Error adding move to {from_team.abbrev}: {from_error}" - to_success, to_error = await to_builder.add_move(to_move, check_pending_transactions=False) + to_success, to_error = await to_builder.add_move( + to_move, check_pending_transactions=False + ) if not to_success: # Rollback both if second failed from_builder.remove_move(player.id) @@ -332,15 +372,13 @@ class TradeBuilder: to_participant.moves_receiving.remove(trade_move) return False, f"Error adding move to {to_team.abbrev}: {to_error}" - logger.info(f"Added player move to trade {self.trade_id}: {trade_move.description}") + logger.info( + f"Added player move to trade {self.trade_id}: {trade_move.description}" + ) return True, "" async def add_supplementary_move( - self, - team: Team, - player: Player, - from_roster: RosterType, - to_roster: RosterType + self, team: Team, player: Player, from_roster: RosterType, to_roster: RosterType ) -> tuple[bool, str]: """ Add a supplementary move (internal organizational move) for roster legality. @@ -366,7 +404,7 @@ class TradeBuilder: from_team=team, to_team=team, source_team=team, - destination_team=team + destination_team=team, ) # Add to participant's supplementary moves @@ -379,16 +417,20 @@ class TradeBuilder: from_roster=from_roster, to_roster=to_roster, from_team=team, - to_team=team + to_team=team, ) # Skip pending transaction check for trade supplementary moves - success, error = await builder.add_move(trans_move, check_pending_transactions=False) + success, error = await builder.add_move( + trans_move, check_pending_transactions=False + ) if not success: participant.supplementary_moves.remove(supp_move) return False, error - logger.info(f"Added supplementary move for {team.abbrev}: {supp_move.description}") + logger.info( + f"Added supplementary move for {team.abbrev}: {supp_move.description}" + ) return True, "" async def remove_move(self, player_id: int) -> tuple[bool, str]: @@ -432,21 +474,41 @@ class TradeBuilder: for builder in self._team_builders.values(): builder.remove_move(player_id) - logger.info(f"Removed move from trade {self.trade_id}: {removed_move.description}") + logger.info( + f"Removed move from trade {self.trade_id}: {removed_move.description}" + ) return True, "" - async def validate_trade(self, next_week: Optional[int] = None) -> TradeValidationResult: + async def validate_trade( + self, next_week: Optional[int] = None + ) -> TradeValidationResult: """ Validate the entire trade including all teams' roster legality. + Validates against next week's projected roster (current roster + pending + transactions), matching the behavior of /dropadd validation. + Args: - next_week: Week to validate for (optional) + next_week: Week to validate for (auto-fetched if not provided) Returns: TradeValidationResult with comprehensive validation """ result = TradeValidationResult() + # Auto-fetch next week so validation includes pending transactions + if next_week is None: + try: + from services.league_service import league_service + + current_state = await league_service.get_current_state() + next_week = (current_state.week + 1) if current_state else 1 + except Exception as e: + logger.warning( + f"Could not determine next week for trade validation: {e}" + ) + next_week = None + # Validate trade structure is_balanced, balance_errors = self.trade.validate_trade_balance() if not is_balanced: @@ -472,13 +534,17 @@ class TradeBuilder: if self.team_count < 2: result.trade_suggestions.append("Add another team to create a trade") - logger.debug(f"Trade validation for {self.trade_id}: Legal={result.is_legal}, Errors={len(result.all_errors)}") + logger.debug( + f"Trade validation for {self.trade_id}: Legal={result.is_legal}, Errors={len(result.all_errors)}" + ) return result def _get_or_create_builder(self, team: Team) -> TransactionBuilder: """Get or create a transaction builder for a team.""" if team.id not in self._team_builders: - self._team_builders[team.id] = TransactionBuilder(team, self.trade.initiated_by, self.trade.season) + self._team_builders[team.id] = TransactionBuilder( + team, self.trade.initiated_by, self.trade.season + ) return self._team_builders[team.id] def clear_trade(self) -> None: @@ -592,4 +658,4 @@ def clear_trade_builder_by_team(team_id: int) -> bool: def get_active_trades() -> Dict[str, TradeBuilder]: """Get all active trade builders (for debugging/admin purposes).""" - return _active_trade_builders.copy() \ No newline at end of file + return _active_trade_builders.copy()