diff --git a/services/trade_builder.py b/services/trade_builder.py index 879959f..aa05b37 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: @@ -24,6 +29,7 @@ class TradeValidationResult: def __init__(self): self.is_legal: bool = True self.participant_validations: Dict[int, RosterValidationResult] = {} + self.team_abbrevs: Dict[int, str] = {} # team_id -> abbreviation self.trade_errors: List[str] = [] self.trade_warnings: List[str] = [] self.trade_suggestions: List[str] = [] @@ -32,27 +38,35 @@ class TradeValidationResult: def all_errors(self) -> List[str]: """Get all errors including trade-level and roster-level errors.""" errors = self.trade_errors.copy() - for validation in self.participant_validations.values(): - errors.extend(validation.errors) + for team_id, validation in self.participant_validations.items(): + abbrev = self.team_abbrevs.get(team_id, "???") + for error in validation.errors: + errors.append(f"[{abbrev}] {error}") return errors @property def all_warnings(self) -> List[str]: """Get all warnings across trade and roster levels.""" warnings = self.trade_warnings.copy() - for validation in self.participant_validations.values(): - warnings.extend(validation.warnings) + for team_id, validation in self.participant_validations.items(): + abbrev = self.team_abbrevs.get(team_id, "???") + for warning in validation.warnings: + warnings.append(f"[{abbrev}] {warning}") return warnings @property def all_suggestions(self) -> List[str]: """Get all suggestions across trade and roster levels.""" suggestions = self.trade_suggestions.copy() - for validation in self.participant_validations.values(): - suggestions.extend(validation.suggestions) + for team_id, validation in self.participant_validations.items(): + abbrev = self.team_abbrevs.get(team_id, "???") + for suggestion in validation.suggestions: + suggestions.append(f"[{abbrev}] {suggestion}") 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 +78,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 +98,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 +110,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 +148,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 +165,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 +187,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 +213,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 +240,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 +263,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 +280,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 +296,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 +314,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 +327,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 +346,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 +355,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 +379,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 +411,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 +424,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 +481,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: @@ -456,6 +525,7 @@ class TradeBuilder: # Validate each team's roster after the trade for participant in self.trade.participants: team_id = participant.team.id + result.team_abbrevs[team_id] = participant.team.abbrev if team_id in self._team_builders: builder = self._team_builders[team_id] roster_validation = await builder.validate_transaction(next_week) @@ -472,13 +542,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 +666,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() diff --git a/tests/test_services_trade_builder.py b/tests/test_services_trade_builder.py index dc2f238..4b9ac99 100644 --- a/tests/test_services_trade_builder.py +++ b/tests/test_services_trade_builder.py @@ -4,6 +4,7 @@ Tests for trade builder service. Tests the TradeBuilder service functionality including multi-team management, move validation, and trade validation logic. """ + import pytest from unittest.mock import AsyncMock, MagicMock, patch @@ -109,7 +110,7 @@ class TestTradeBuilder: self.player1.team_id = self.team1.id # Mock team_service to return team1 for this player - with patch('services.trade_builder.team_service') as mock_team_service: + with patch("services.trade_builder.team_service") as mock_team_service: mock_team_service.get_team = AsyncMock(return_value=self.team1) # Don't mock is_same_organization - let the real method work @@ -119,7 +120,7 @@ class TestTradeBuilder: from_team=self.team1, to_team=self.team2, from_roster=RosterType.MAJOR_LEAGUE, - to_roster=RosterType.MAJOR_LEAGUE + to_roster=RosterType.MAJOR_LEAGUE, ) assert success @@ -141,7 +142,7 @@ class TestTradeBuilder: from_team=self.team2, to_team=self.team1, from_roster=RosterType.MAJOR_LEAGUE, - to_roster=RosterType.MAJOR_LEAGUE + to_roster=RosterType.MAJOR_LEAGUE, ) assert not success @@ -156,9 +157,7 @@ class TestTradeBuilder: # Create a player on Free Agency fa_player = PlayerFactory.create( - id=100, - name="FA Player", - team_id=get_config().free_agent_team_id + id=100, name="FA Player", team_id=get_config().free_agent_team_id ) # Try to add player from FA (should fail) @@ -167,7 +166,7 @@ class TestTradeBuilder: from_team=self.team1, to_team=self.team2, from_roster=RosterType.MAJOR_LEAGUE, - to_roster=RosterType.MAJOR_LEAGUE + to_roster=RosterType.MAJOR_LEAGUE, ) assert not success @@ -182,9 +181,7 @@ class TestTradeBuilder: # Create a player without a team no_team_player = PlayerFactory.create( - id=101, - name="No Team Player", - team_id=None + id=101, name="No Team Player", team_id=None ) # Try to add player without team (should fail) @@ -193,7 +190,7 @@ class TestTradeBuilder: from_team=self.team1, to_team=self.team2, from_roster=RosterType.MAJOR_LEAGUE, - to_roster=RosterType.MAJOR_LEAGUE + to_roster=RosterType.MAJOR_LEAGUE, ) assert not success @@ -208,24 +205,22 @@ class TestTradeBuilder: # Create a player on team3 (not in trade) player_on_team3 = PlayerFactory.create( - id=102, - name="Team3 Player", - team_id=self.team3.id + id=102, name="Team3 Player", team_id=self.team3.id ) # Mock team_service to return team3 for this player - with patch('services.trade_builder.team_service') as mock_team_service: + with patch("services.trade_builder.team_service") as mock_team_service: mock_team_service.get_team = AsyncMock(return_value=self.team3) # Mock is_same_organization to return False (different organization, sync method) - with patch('models.team.Team.is_same_organization', return_value=False): + with patch("models.team.Team.is_same_organization", return_value=False): # Try to add player from team3 claiming it's from team1 (should fail) success, error = await builder.add_player_move( player=player_on_team3, from_team=self.team1, to_team=self.team2, from_roster=RosterType.MAJOR_LEAGUE, - to_roster=RosterType.MAJOR_LEAGUE + to_roster=RosterType.MAJOR_LEAGUE, ) assert not success @@ -241,26 +236,24 @@ class TestTradeBuilder: # Create a player on team1's minor league affiliate player_on_team1_mil = PlayerFactory.create( - id=103, - name="Team1 MiL Player", - team_id=999 # Some MiL team ID + id=103, name="Team1 MiL Player", team_id=999 # Some MiL team ID ) # Mock team_service to return the MiL team mil_team = TeamFactory.create(id=999, abbrev="WVMiL", sname="West Virginia MiL") - with patch('services.trade_builder.team_service') as mock_team_service: + with patch("services.trade_builder.team_service") as mock_team_service: mock_team_service.get_team = AsyncMock(return_value=mil_team) # Mock is_same_organization to return True (same organization, sync method) - with patch('models.team.Team.is_same_organization', return_value=True): + with patch("models.team.Team.is_same_organization", return_value=True): # Add player from WVMiL (should succeed because it's same organization as WV) success, error = await builder.add_player_move( player=player_on_team1_mil, from_team=self.team1, to_team=self.team2, from_roster=RosterType.MINOR_LEAGUE, - to_roster=RosterType.MAJOR_LEAGUE + to_roster=RosterType.MAJOR_LEAGUE, ) assert success @@ -278,7 +271,7 @@ class TestTradeBuilder: team=self.team1, player=self.player1, from_roster=RosterType.MINOR_LEAGUE, - to_roster=RosterType.MAJOR_LEAGUE + to_roster=RosterType.MAJOR_LEAGUE, ) assert success @@ -293,7 +286,7 @@ class TestTradeBuilder: team=self.team3, player=self.player2, from_roster=RosterType.MINOR_LEAGUE, - to_roster=RosterType.MAJOR_LEAGUE + to_roster=RosterType.MAJOR_LEAGUE, ) assert not success @@ -309,7 +302,7 @@ class TestTradeBuilder: self.player1.team_id = self.team1.id # Mock team_service for adding the player - with patch('services.trade_builder.team_service') as mock_team_service: + with patch("services.trade_builder.team_service") as mock_team_service: mock_team_service.get_team = AsyncMock(return_value=self.team1) # Add a player move @@ -318,7 +311,7 @@ class TestTradeBuilder: from_team=self.team1, to_team=self.team2, from_roster=RosterType.MAJOR_LEAGUE, - to_roster=RosterType.MAJOR_LEAGUE + to_roster=RosterType.MAJOR_LEAGUE, ) assert not builder.is_empty @@ -347,7 +340,7 @@ class TestTradeBuilder: await builder.add_team(self.team2) # Mock the transaction builders - with patch.object(builder, '_get_or_create_builder') as mock_get_builder: + with patch.object(builder, "_get_or_create_builder") as mock_get_builder: mock_builder1 = MagicMock() mock_builder2 = MagicMock() @@ -360,7 +353,7 @@ class TestTradeBuilder: minor_league_count=5, warnings=[], errors=[], - suggestions=[] + suggestions=[], ) mock_builder1.validate_transaction = AsyncMock(return_value=valid_result) @@ -391,7 +384,7 @@ class TestTradeBuilder: await builder.add_team(self.team2) # Mock the transaction builders - with patch.object(builder, '_get_or_create_builder') as mock_get_builder: + with patch.object(builder, "_get_or_create_builder") as mock_get_builder: mock_builder1 = MagicMock() mock_builder2 = MagicMock() @@ -404,7 +397,7 @@ class TestTradeBuilder: minor_league_count=5, warnings=[], errors=[], - suggestions=[] + suggestions=[], ) mock_builder1.validate_transaction = AsyncMock(return_value=valid_result) @@ -440,7 +433,7 @@ class TestTradeBuilder: return self.team2 return None - with patch('services.trade_builder.team_service') as mock_team_service: + with patch("services.trade_builder.team_service") as mock_team_service: mock_team_service.get_team = AsyncMock(side_effect=get_team_side_effect) # Add balanced moves - no need to mock is_same_organization @@ -449,7 +442,7 @@ class TestTradeBuilder: from_team=self.team1, to_team=self.team2, from_roster=RosterType.MAJOR_LEAGUE, - to_roster=RosterType.MAJOR_LEAGUE + to_roster=RosterType.MAJOR_LEAGUE, ) await builder.add_player_move( @@ -457,7 +450,7 @@ class TestTradeBuilder: from_team=self.team2, to_team=self.team1, from_roster=RosterType.MAJOR_LEAGUE, - to_roster=RosterType.MAJOR_LEAGUE + to_roster=RosterType.MAJOR_LEAGUE, ) # Validate balanced trade @@ -829,7 +822,9 @@ class TestTradeBuilderCache: """ user_id = 12345 team1 = TeamFactory.west_virginia() - team3 = TeamFactory.create(id=999, abbrev="POR", name="Portland") # Non-participant + team3 = TeamFactory.create( + id=999, abbrev="POR", name="Portland" + ) # Non-participant # Create builder with team1 get_trade_builder(user_id, team1) @@ -955,7 +950,9 @@ class TestTradeBuilderCache: This ensures proper error handling when a GM not in the trade tries to clear it. """ - team3 = TeamFactory.create(id=999, abbrev="POR", name="Portland") # Non-participant + team3 = TeamFactory.create( + id=999, abbrev="POR", name="Portland" + ) # Non-participant result = clear_trade_builder_by_team(team3.id) assert result is False @@ -982,7 +979,7 @@ class TestTradeValidationResult: minor_league_count=5, warnings=["Team1 warning"], errors=["Team1 error"], - suggestions=["Team1 suggestion"] + suggestions=["Team1 suggestion"], ) team2_validation = RosterValidationResult( @@ -991,28 +988,30 @@ class TestTradeValidationResult: minor_league_count=4, warnings=[], errors=[], - suggestions=[] + suggestions=[], ) result.participant_validations[1] = team1_validation result.participant_validations[2] = team2_validation + result.team_abbrevs[1] = "TM1" + result.team_abbrevs[2] = "TM2" result.is_legal = False # One team has errors - # Test aggregated results + # Test aggregated results - roster errors are prefixed with team abbrev all_errors = result.all_errors assert len(all_errors) == 3 # 2 trade + 1 team assert "Trade error 1" in all_errors - assert "Team1 error" in all_errors + assert "[TM1] Team1 error" in all_errors all_warnings = result.all_warnings assert len(all_warnings) == 2 # 1 trade + 1 team assert "Trade warning 1" in all_warnings - assert "Team1 warning" in all_warnings + assert "[TM1] Team1 warning" in all_warnings all_suggestions = result.all_suggestions assert len(all_suggestions) == 2 # 1 trade + 1 team assert "Trade suggestion 1" in all_suggestions - assert "Team1 suggestion" in all_suggestions + assert "[TM1] Team1 suggestion" in all_suggestions # Test participant validation lookup team1_val = result.get_participant_validation(1) @@ -1029,4 +1028,4 @@ class TestTradeValidationResult: assert len(result.all_errors) == 0 assert len(result.all_warnings) == 0 assert len(result.all_suggestions) == 0 - assert len(result.participant_validations) == 0 \ No newline at end of file + assert len(result.participant_validations) == 0