cal/major-domo-v2
1
0
Fork 0
Code Issues 6 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix: prefix trade validation errors with team abbreviation
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m5s
Details

Browse Source 
Roster-level errors, warnings, and suggestions now display as
"[BSG] Too many ML players" so users know which team each issue
applies to.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Cal Corum 2026-03-08 11:37:39 -05:00
parent f7a65706a1
commit 8ee35e564c
2 changed files with 56 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
services/trade_builder.py
View File

@ -24,6 +24,7 @@ class TradeValidationResult:
def __init__(self): def __init__(self):
self.is_legal: bool = True self.is_legal: bool = True
self.participant_validations: Dict[int, RosterValidationResult] = {} self.participant_validations: Dict[int, RosterValidationResult] = {}
self.team_abbrevs: Dict[int, str] = {} # team_id -> abbreviation
self.trade_errors: List[str] = [] self.trade_errors: List[str] = []
self.trade_warnings: List[str] = [] self.trade_warnings: List[str] = []
self.trade_suggestions: List[str] = [] self.trade_suggestions: List[str] = []
@ -32,24 +33,30 @@ class TradeValidationResult:
def all_errors(self) -> List[str]: def all_errors(self) -> List[str]:
"""Get all errors including trade-level and roster-level errors.""" """Get all errors including trade-level and roster-level errors."""
errors = self.trade_errors.copy() errors = self.trade_errors.copy()
for validation in self.participant_validations.values(): for team_id, validation in self.participant_validations.items():
errors.extend(validation.errors) abbrev = self.team_abbrevs.get(team_id, "???")
for error in validation.errors:
errors.append(f"[{abbrev}] {error}")
return errors return errors
@property @property
def all_warnings(self) -> List[str]: def all_warnings(self) -> List[str]:
"""Get all warnings across trade and roster levels.""" """Get all warnings across trade and roster levels."""
warnings = self.trade_warnings.copy() warnings = self.trade_warnings.copy()
for validation in self.participant_validations.values(): for team_id, validation in self.participant_validations.items():
warnings.extend(validation.warnings) abbrev = self.team_abbrevs.get(team_id, "???")
for warning in validation.warnings:
warnings.append(f"[{abbrev}] {warning}")
return warnings return warnings
@property @property
def all_suggestions(self) -> List[str]: def all_suggestions(self) -> List[str]:
"""Get all suggestions across trade and roster levels.""" """Get all suggestions across trade and roster levels."""
suggestions = self.trade_suggestions.copy() suggestions = self.trade_suggestions.copy()
for validation in self.participant_validations.values(): for team_id, validation in self.participant_validations.items():
suggestions.extend(validation.suggestions) abbrev = self.team_abbrevs.get(team_id, "???")
for suggestion in validation.suggestions:
suggestions.append(f"[{abbrev}] {suggestion}")
return suggestions return suggestions
def get_participant_validation(self, team_id: int) -> Optional[RosterValidationResult]: def get_participant_validation(self, team_id: int) -> Optional[RosterValidationResult]:
@ -456,6 +463,7 @@ class TradeBuilder:
# Validate each team's roster after the trade # Validate each team's roster after the trade
for participant in self.trade.participants: for participant in self.trade.participants:
team_id = participant.team.id team_id = participant.team.id
result.team_abbrevs[team_id] = participant.team.abbrev
if team_id in self._team_builders: if team_id in self._team_builders:
builder = self._team_builders[team_id] builder = self._team_builders[team_id]
roster_validation = await builder.validate_transaction(next_week) roster_validation = await builder.validate_transaction(next_week)

83
tests/test_services_trade_builder.py
View File

@ -4,6 +4,7 @@ Tests for trade builder service.
Tests the TradeBuilder service functionality including multi-team management, Tests the TradeBuilder service functionality including multi-team management,
move validation, and trade validation logic. move validation, and trade validation logic.
""" """
import pytest import pytest
from unittest.mock import AsyncMock, MagicMock, patch from unittest.mock import AsyncMock, MagicMock, patch
@ -109,7 +110,7 @@ class TestTradeBuilder:
self.player1.team_id = self.team1.id self.player1.team_id = self.team1.id
# Mock team_service to return team1 for this player # 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) mock_team_service.get_team = AsyncMock(return_value=self.team1)
# Don't mock is_same_organization - let the real method work # Don't mock is_same_organization - let the real method work
@ -119,7 +120,7 @@ class TestTradeBuilder:
from_team=self.team1, from_team=self.team1,
to_team=self.team2, to_team=self.team2,
from_roster=RosterType.MAJOR_LEAGUE, from_roster=RosterType.MAJOR_LEAGUE,
to_roster=RosterType.MAJOR_LEAGUE to_roster=RosterType.MAJOR_LEAGUE,
) )
assert success assert success
@ -141,7 +142,7 @@ class TestTradeBuilder:
from_team=self.team2, from_team=self.team2,
to_team=self.team1, to_team=self.team1,
from_roster=RosterType.MAJOR_LEAGUE, from_roster=RosterType.MAJOR_LEAGUE,
to_roster=RosterType.MAJOR_LEAGUE to_roster=RosterType.MAJOR_LEAGUE,
) )
assert not success assert not success
@ -156,9 +157,7 @@ class TestTradeBuilder:
# Create a player on Free Agency # Create a player on Free Agency
fa_player = PlayerFactory.create( fa_player = PlayerFactory.create(
id=100, id=100, name="FA Player", team_id=get_config().free_agent_team_id
name="FA Player",
team_id=get_config().free_agent_team_id
) )
# Try to add player from FA (should fail) # Try to add player from FA (should fail)
@ -167,7 +166,7 @@ class TestTradeBuilder:
from_team=self.team1, from_team=self.team1,
to_team=self.team2, to_team=self.team2,
from_roster=RosterType.MAJOR_LEAGUE, from_roster=RosterType.MAJOR_LEAGUE,
to_roster=RosterType.MAJOR_LEAGUE to_roster=RosterType.MAJOR_LEAGUE,
) )
assert not success assert not success
@ -182,9 +181,7 @@ class TestTradeBuilder:
# Create a player without a team # Create a player without a team
no_team_player = PlayerFactory.create( no_team_player = PlayerFactory.create(
id=101, id=101, name="No Team Player", team_id=None
name="No Team Player",
team_id=None
) )
# Try to add player without team (should fail) # Try to add player without team (should fail)
@ -193,7 +190,7 @@ class TestTradeBuilder:
from_team=self.team1, from_team=self.team1,
to_team=self.team2, to_team=self.team2,
from_roster=RosterType.MAJOR_LEAGUE, from_roster=RosterType.MAJOR_LEAGUE,
to_roster=RosterType.MAJOR_LEAGUE to_roster=RosterType.MAJOR_LEAGUE,
) )
assert not success assert not success
@ -208,24 +205,22 @@ class TestTradeBuilder:
# Create a player on team3 (not in trade) # Create a player on team3 (not in trade)
player_on_team3 = PlayerFactory.create( player_on_team3 = PlayerFactory.create(
id=102, id=102, name="Team3 Player", team_id=self.team3.id
name="Team3 Player",
team_id=self.team3.id
) )
# Mock team_service to return team3 for this player # 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_team_service.get_team = AsyncMock(return_value=self.team3)
# Mock is_same_organization to return False (different organization, sync method) # 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) # Try to add player from team3 claiming it's from team1 (should fail)
success, error = await builder.add_player_move( success, error = await builder.add_player_move(
player=player_on_team3, player=player_on_team3,
from_team=self.team1, from_team=self.team1,
to_team=self.team2, to_team=self.team2,
from_roster=RosterType.MAJOR_LEAGUE, from_roster=RosterType.MAJOR_LEAGUE,
to_roster=RosterType.MAJOR_LEAGUE to_roster=RosterType.MAJOR_LEAGUE,
) )
assert not success assert not success
@ -241,26 +236,24 @@ class TestTradeBuilder:
# Create a player on team1's minor league affiliate # Create a player on team1's minor league affiliate
player_on_team1_mil = PlayerFactory.create( player_on_team1_mil = PlayerFactory.create(
id=103, id=103, name="Team1 MiL Player", team_id=999 # Some MiL team ID
name="Team1 MiL Player",
team_id=999 # Some MiL team ID
) )
# Mock team_service to return the MiL team # Mock team_service to return the MiL team
mil_team = TeamFactory.create(id=999, abbrev="WVMiL", sname="West Virginia MiL") 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_team_service.get_team = AsyncMock(return_value=mil_team)
# Mock is_same_organization to return True (same organization, sync method) # 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) # Add player from WVMiL (should succeed because it's same organization as WV)
success, error = await builder.add_player_move( success, error = await builder.add_player_move(
player=player_on_team1_mil, player=player_on_team1_mil,
from_team=self.team1, from_team=self.team1,
to_team=self.team2, to_team=self.team2,
from_roster=RosterType.MINOR_LEAGUE, from_roster=RosterType.MINOR_LEAGUE,
to_roster=RosterType.MAJOR_LEAGUE to_roster=RosterType.MAJOR_LEAGUE,
) )
assert success assert success
@ -278,7 +271,7 @@ class TestTradeBuilder:
team=self.team1, team=self.team1,
player=self.player1, player=self.player1,
from_roster=RosterType.MINOR_LEAGUE, from_roster=RosterType.MINOR_LEAGUE,
to_roster=RosterType.MAJOR_LEAGUE to_roster=RosterType.MAJOR_LEAGUE,
) )
assert success assert success
@ -293,7 +286,7 @@ class TestTradeBuilder:
team=self.team3, team=self.team3,
player=self.player2, player=self.player2,
from_roster=RosterType.MINOR_LEAGUE, from_roster=RosterType.MINOR_LEAGUE,
to_roster=RosterType.MAJOR_LEAGUE to_roster=RosterType.MAJOR_LEAGUE,
) )
assert not success assert not success
@ -309,7 +302,7 @@ class TestTradeBuilder:
self.player1.team_id = self.team1.id self.player1.team_id = self.team1.id
# Mock team_service for adding the player # 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) mock_team_service.get_team = AsyncMock(return_value=self.team1)
# Add a player move # Add a player move
@ -318,7 +311,7 @@ class TestTradeBuilder:
from_team=self.team1, from_team=self.team1,
to_team=self.team2, to_team=self.team2,
from_roster=RosterType.MAJOR_LEAGUE, from_roster=RosterType.MAJOR_LEAGUE,
to_roster=RosterType.MAJOR_LEAGUE to_roster=RosterType.MAJOR_LEAGUE,
) )
assert not builder.is_empty assert not builder.is_empty
@ -347,7 +340,7 @@ class TestTradeBuilder:
await builder.add_team(self.team2) await builder.add_team(self.team2)
# Mock the transaction builders # 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_builder1 = MagicMock()
mock_builder2 = MagicMock() mock_builder2 = MagicMock()
@ -360,7 +353,7 @@ class TestTradeBuilder:
minor_league_count=5, minor_league_count=5,
warnings=[], warnings=[],
errors=[], errors=[],
suggestions=[] suggestions=[],
) )
mock_builder1.validate_transaction = AsyncMock(return_value=valid_result) mock_builder1.validate_transaction = AsyncMock(return_value=valid_result)
@ -391,7 +384,7 @@ class TestTradeBuilder:
await builder.add_team(self.team2) await builder.add_team(self.team2)
# Mock the transaction builders # 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_builder1 = MagicMock()
mock_builder2 = MagicMock() mock_builder2 = MagicMock()
@ -404,7 +397,7 @@ class TestTradeBuilder:
minor_league_count=5, minor_league_count=5,
warnings=[], warnings=[],
errors=[], errors=[],
suggestions=[] suggestions=[],
) )
mock_builder1.validate_transaction = AsyncMock(return_value=valid_result) mock_builder1.validate_transaction = AsyncMock(return_value=valid_result)
@ -440,7 +433,7 @@ class TestTradeBuilder:
return self.team2 return self.team2
return None 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) mock_team_service.get_team = AsyncMock(side_effect=get_team_side_effect)
# Add balanced moves - no need to mock is_same_organization # Add balanced moves - no need to mock is_same_organization
@ -449,7 +442,7 @@ class TestTradeBuilder:
from_team=self.team1, from_team=self.team1,
to_team=self.team2, to_team=self.team2,
from_roster=RosterType.MAJOR_LEAGUE, from_roster=RosterType.MAJOR_LEAGUE,
to_roster=RosterType.MAJOR_LEAGUE to_roster=RosterType.MAJOR_LEAGUE,
) )
await builder.add_player_move( await builder.add_player_move(
@ -457,7 +450,7 @@ class TestTradeBuilder:
from_team=self.team2, from_team=self.team2,
to_team=self.team1, to_team=self.team1,
from_roster=RosterType.MAJOR_LEAGUE, from_roster=RosterType.MAJOR_LEAGUE,
to_roster=RosterType.MAJOR_LEAGUE to_roster=RosterType.MAJOR_LEAGUE,
) )
# Validate balanced trade # Validate balanced trade
@ -829,7 +822,9 @@ class TestTradeBuilderCache:
""" """
user_id = 12345 user_id = 12345
team1 = TeamFactory.west_virginia() 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 # Create builder with team1
get_trade_builder(user_id, 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. 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) result = clear_trade_builder_by_team(team3.id)
assert result is False assert result is False
@ -982,7 +979,7 @@ class TestTradeValidationResult:
minor_league_count=5, minor_league_count=5,
warnings=["Team1 warning"], warnings=["Team1 warning"],
errors=["Team1 error"], errors=["Team1 error"],
suggestions=["Team1 suggestion"] suggestions=["Team1 suggestion"],
) )
team2_validation = RosterValidationResult( team2_validation = RosterValidationResult(
@ -991,28 +988,30 @@ class TestTradeValidationResult:
minor_league_count=4, minor_league_count=4,
warnings=[], warnings=[],
errors=[], errors=[],
suggestions=[] suggestions=[],
) )
result.participant_validations[1] = team1_validation result.participant_validations[1] = team1_validation
result.participant_validations[2] = team2_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 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 all_errors = result.all_errors
assert len(all_errors) == 3 # 2 trade + 1 team assert len(all_errors) == 3 # 2 trade + 1 team
assert "Trade error 1" in all_errors 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 all_warnings = result.all_warnings
assert len(all_warnings) == 2 # 1 trade + 1 team assert len(all_warnings) == 2 # 1 trade + 1 team
assert "Trade warning 1" in all_warnings 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 all_suggestions = result.all_suggestions
assert len(all_suggestions) == 2 # 1 trade + 1 team assert len(all_suggestions) == 2 # 1 trade + 1 team
assert "Trade suggestion 1" in all_suggestions assert "Trade suggestion 1" in all_suggestions
assert "Team1 suggestion" in all_suggestions assert "[TM1] Team1 suggestion" in all_suggestions
# Test participant validation lookup # Test participant validation lookup
team1_val = result.get_participant_validation(1) team1_val = result.get_participant_validation(1)