From c70669d4744a2913d2eb5890e36503e4fa44f23e Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Mon, 2 Mar 2026 14:56:14 -0600 Subject: [PATCH] fix: refresh roster data before validation to prevent stale cache TransactionBuilder cached roster data indefinitely via _roster_loaded flag, causing validation to use stale counts when builders persisted across multiple /ilmove invocations. Now validate_transaction() always fetches fresh roster data. Also adds dependency injection for roster_service to improve testability. Co-Authored-By: Claude Opus 4.6 --- services/transaction_builder.py | 28 +- tests/test_services_transaction_builder.py | 670 +++++++++++---------- 2 files changed, 383 insertions(+), 315 deletions(-) diff --git a/services/transaction_builder.py b/services/transaction_builder.py index 0ee9e2d..c38145a 100644 --- a/services/transaction_builder.py +++ b/services/transaction_builder.py @@ -174,7 +174,13 @@ class RosterValidationResult: class TransactionBuilder: """Interactive transaction builder for complex multi-move transactions.""" - def __init__(self, team: Team, user_id: int, season: int = get_config().sba_season): + def __init__( + self, + team: Team, + user_id: int, + season: int = get_config().sba_season, + roster_svc=None, + ): """ Initialize transaction builder. @@ -182,12 +188,14 @@ class TransactionBuilder: team: Team making the transaction user_id: Discord user ID of the GM season: Season number + roster_svc: RosterService instance (defaults to global roster_service) """ self.team = team self.user_id = user_id self.season = season self.moves: List[TransactionMove] = [] self.created_at = datetime.now(timezone.utc) + self._roster_svc = roster_svc or roster_service # Cache for roster data self._current_roster: Optional[TeamRoster] = None @@ -201,13 +209,19 @@ class TransactionBuilder: f"TransactionBuilder initialized for {team.abbrev} by user {user_id}" ) - async def load_roster_data(self) -> None: - """Load current roster data for the team.""" - if self._roster_loaded: + async def load_roster_data(self, force_refresh: bool = False) -> None: + """Load current roster data for the team. + + Args: + force_refresh: If True, bypass cache and fetch fresh data from API. + """ + if self._roster_loaded and not force_refresh: return try: - self._current_roster = await roster_service.get_current_roster(self.team.id) + self._current_roster = await self._roster_svc.get_current_roster( + self.team.id + ) self._roster_loaded = True logger.debug(f"Loaded roster data for team {self.team.abbrev}") except Exception as e: @@ -353,7 +367,9 @@ class TransactionBuilder: Returns: RosterValidationResult with validation details """ - await self.load_roster_data() + # Always refresh roster data before validation to prevent stale cache + # from causing incorrect roster counts (e.g. after a previous /ilmove submission) + await self.load_roster_data(force_refresh=True) # Load existing transactions if next_week is provided if next_week is not None: diff --git a/tests/test_services_transaction_builder.py b/tests/test_services_transaction_builder.py index 8b3dbcb..0fc1e72 100644 --- a/tests/test_services_transaction_builder.py +++ b/tests/test_services_transaction_builder.py @@ -3,6 +3,7 @@ Tests for TransactionBuilder service Validates transaction building, roster validation, and move management. """ + import pytest from unittest.mock import AsyncMock, MagicMock, patch @@ -12,7 +13,7 @@ from services.transaction_builder import ( RosterType, RosterValidationResult, get_transaction_builder, - clear_transaction_builder + clear_transaction_builder, ) from models.team import Team from models.player import Player @@ -28,92 +29,99 @@ class TestTransactionBuilder: """Create a mock team for testing.""" return Team( id=499, - abbrev='WV', - sname='Black Bears', - lname='West Virginia Black Bears', + abbrev="WV", + sname="Black Bears", + lname="West Virginia Black Bears", season=12, - salary_cap=50.0 # Set high cap to avoid sWAR validation failures in roster tests + salary_cap=50.0, # Set high cap to avoid sWAR validation failures in roster tests ) @pytest.fixture def mock_player(self): """Create a mock player for testing.""" - return Player( - id=12472, - name='Test Player', - wara=2.5, - season=12, - pos_1='OF' - ) - + return Player(id=12472, name="Test Player", wara=2.5, season=12, pos_1="OF") + @pytest.fixture def mock_roster(self): """Create a mock roster for testing.""" # Create roster players ml_players = [] for i in range(24): # 24 ML players (under limit) - ml_players.append(Player( - id=1000 + i, - name=f'ML Player {i}', - wara=1.5, - season=12, - team_id=499, - team=None, - image=None, - image2=None, - vanity_card=None, - headshot=None, - pos_1='OF', - pitcher_injury=None, - injury_rating=None, - il_return=None, - demotion_week=None, - last_game=None, - last_game2=None, - strat_code=None, - bbref_id=None, - sbaplayer=None - )) - + ml_players.append( + Player( + id=1000 + i, + name=f"ML Player {i}", + wara=1.5, + season=12, + team_id=499, + team=None, + image=None, + image2=None, + vanity_card=None, + headshot=None, + pos_1="OF", + pitcher_injury=None, + injury_rating=None, + il_return=None, + demotion_week=None, + last_game=None, + last_game2=None, + strat_code=None, + bbref_id=None, + sbaplayer=None, + ) + ) + mil_players = [] for i in range(6): # 6 MiL players (at limit) - mil_players.append(Player( - id=2000 + i, - name=f'MiL Player {i}', - wara=0.5, - season=12, - team_id=499, - team=None, - image=None, - image2=None, - vanity_card=None, - headshot=None, - pos_1='OF', - pitcher_injury=None, - injury_rating=None, - il_return=None, - demotion_week=None, - last_game=None, - last_game2=None, - strat_code=None, - bbref_id=None, - sbaplayer=None - )) - + mil_players.append( + Player( + id=2000 + i, + name=f"MiL Player {i}", + wara=0.5, + season=12, + team_id=499, + team=None, + image=None, + image2=None, + vanity_card=None, + headshot=None, + pos_1="OF", + pitcher_injury=None, + injury_rating=None, + il_return=None, + demotion_week=None, + last_game=None, + last_game2=None, + strat_code=None, + bbref_id=None, + sbaplayer=None, + ) + ) + return TeamRoster( team_id=499, - team_abbrev='WV', + team_abbrev="WV", week=10, season=12, active_players=ml_players, - minor_league_players=mil_players + minor_league_players=mil_players, ) - + @pytest.fixture - def builder(self, mock_team): - """Create a TransactionBuilder for testing.""" - return TransactionBuilder(mock_team, user_id=123456789, season=12) - + def mock_roster_service(self, mock_roster): + """Create a mock roster service that returns mock_roster.""" + svc = AsyncMock() + svc.get_current_roster.return_value = mock_roster + return svc + + @pytest.fixture + def builder(self, mock_team, mock_roster_service): + """Create a TransactionBuilder for testing with injected roster service.""" + return TransactionBuilder( + mock_team, user_id=123456789, season=12, roster_svc=mock_roster_service + ) + def test_builder_initialization(self, builder, mock_team): """Test transaction builder initialization.""" assert builder.team == mock_team @@ -122,7 +130,7 @@ class TestTransactionBuilder: assert builder.is_empty is True assert builder.move_count == 0 assert len(builder.moves) == 0 - + @pytest.mark.asyncio async def test_add_move_success(self, builder, mock_player): """Test successfully adding a move.""" @@ -130,11 +138,13 @@ class TestTransactionBuilder: player=mock_player, from_roster=RosterType.FREE_AGENCY, to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team + to_team=builder.team, ) # Skip pending transaction check for unit tests - success, error_message = await builder.add_move(move, check_pending_transactions=False) + success, error_message = await builder.add_move( + move, check_pending_transactions=False + ) assert success is True assert error_message == "" @@ -149,18 +159,22 @@ class TestTransactionBuilder: player=mock_player, from_roster=RosterType.FREE_AGENCY, to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team + to_team=builder.team, ) move2 = TransactionMove( player=mock_player, from_roster=RosterType.MAJOR_LEAGUE, to_roster=RosterType.FREE_AGENCY, - from_team=builder.team + from_team=builder.team, ) - success1, error_message1 = await builder.add_move(move1, check_pending_transactions=False) - success2, error_message2 = await builder.add_move(move2, check_pending_transactions=False) + success1, error_message1 = await builder.add_move( + move1, check_pending_transactions=False + ) + success2, error_message2 = await builder.add_move( + move2, check_pending_transactions=False + ) assert success1 is True assert error_message1 == "" @@ -176,10 +190,12 @@ class TestTransactionBuilder: from_roster=RosterType.MAJOR_LEAGUE, to_roster=RosterType.MAJOR_LEAGUE, # Same roster from_team=builder.team, - to_team=builder.team # Same team - should fail when roster is also same + to_team=builder.team, # Same team - should fail when roster is also same ) - success, error_message = await builder.add_move(move, check_pending_transactions=False) + success, error_message = await builder.add_move( + move, check_pending_transactions=False + ) assert success is False assert "already in that location" in error_message @@ -187,17 +203,21 @@ class TestTransactionBuilder: assert builder.is_empty is True @pytest.mark.asyncio - async def test_add_move_same_team_different_roster_succeeds(self, builder, mock_player): + async def test_add_move_same_team_different_roster_succeeds( + self, builder, mock_player + ): """Test that adding a move where teams are same but rosters are different succeeds.""" move = TransactionMove( player=mock_player, from_roster=RosterType.MAJOR_LEAGUE, to_roster=RosterType.MINOR_LEAGUE, # Different roster from_team=builder.team, - to_team=builder.team # Same team - should succeed when rosters differ + to_team=builder.team, # Same team - should succeed when rosters differ ) - success, error_message = await builder.add_move(move, check_pending_transactions=False) + success, error_message = await builder.add_move( + move, check_pending_transactions=False + ) assert success is True assert error_message == "" @@ -208,11 +228,7 @@ class TestTransactionBuilder: async def test_add_move_different_teams_succeeds(self, builder, mock_player): """Test that adding a move where from_team and to_team are different succeeds.""" other_team = Team( - id=500, - abbrev='NY', - sname='Mets', - lname='New York Mets', - season=12 + id=500, abbrev="NY", sname="Mets", lname="New York Mets", season=12 ) move = TransactionMove( @@ -220,10 +236,12 @@ class TestTransactionBuilder: from_roster=RosterType.MAJOR_LEAGUE, to_roster=RosterType.MAJOR_LEAGUE, from_team=other_team, - to_team=builder.team + to_team=builder.team, ) - success, error_message = await builder.add_move(move, check_pending_transactions=False) + success, error_message = await builder.add_move( + move, check_pending_transactions=False + ) assert success is True assert error_message == "" @@ -239,10 +257,12 @@ class TestTransactionBuilder: from_roster=RosterType.FREE_AGENCY, to_roster=RosterType.MAJOR_LEAGUE, from_team=None, - to_team=builder.team + to_team=builder.team, ) - success1, error_message1 = await builder.add_move(move1, check_pending_transactions=False) + success1, error_message1 = await builder.add_move( + move1, check_pending_transactions=False + ) assert success1 is True assert error_message1 == "" @@ -250,11 +270,7 @@ class TestTransactionBuilder: # Create different player for second test other_player = Player( - id=12473, - name='Other Player', - wara=1.5, - season=12, - pos_1='OF' + id=12473, name="Other Player", wara=1.5, season=12, pos_1="OF" ) # From team to FA (to_team=None) @@ -263,10 +279,12 @@ class TestTransactionBuilder: from_roster=RosterType.MAJOR_LEAGUE, to_roster=RosterType.FREE_AGENCY, from_team=builder.team, - to_team=None + to_team=None, ) - success2, error_message2 = await builder.add_move(move2, check_pending_transactions=False) + success2, error_message2 = await builder.add_move( + move2, check_pending_transactions=False + ) assert success2 is True assert error_message2 == "" @@ -277,7 +295,7 @@ class TestTransactionBuilder: player=mock_player, from_roster=RosterType.FREE_AGENCY, to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team + to_team=builder.team, ) success, _ = await builder.add_move(move, check_pending_transactions=False) @@ -289,14 +307,14 @@ class TestTransactionBuilder: assert removed is True assert builder.move_count == 0 assert builder.is_empty is True - + def test_remove_nonexistent_move(self, builder): """Test removing a move that doesn't exist.""" removed = builder.remove_move(99999) - + assert removed is False assert builder.move_count == 0 - + @pytest.mark.asyncio async def test_get_move_for_player(self, builder, mock_player): """Test getting move for a specific player.""" @@ -304,7 +322,7 @@ class TestTransactionBuilder: player=mock_player, from_roster=RosterType.FREE_AGENCY, to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team + to_team=builder.team, ) await builder.add_move(move, check_pending_transactions=False) @@ -322,7 +340,7 @@ class TestTransactionBuilder: player=mock_player, from_roster=RosterType.FREE_AGENCY, to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team + to_team=builder.team, ) success, _ = await builder.add_move(move, check_pending_transactions=False) @@ -335,80 +353,78 @@ class TestTransactionBuilder: assert builder.is_empty is True @pytest.mark.asyncio - async def test_validate_transaction_no_roster(self, builder): + async def test_validate_transaction_no_roster(self, mock_team): """Test validation when roster data cannot be loaded.""" - with patch.object(builder, '_current_roster', None): - with patch.object(builder, '_roster_loaded', True): - validation = await builder.validate_transaction() + failing_svc = AsyncMock() + failing_svc.get_current_roster.return_value = None + builder = TransactionBuilder( + mock_team, user_id=123456789, season=12, roster_svc=failing_svc + ) - assert validation.is_legal is False - assert len(validation.errors) == 1 - assert "Could not load current roster data" in validation.errors[0] + validation = await builder.validate_transaction() + + assert validation.is_legal is False + assert len(validation.errors) == 1 + assert "Could not load current roster data" in validation.errors[0] @pytest.mark.asyncio - async def test_validate_transaction_legal(self, builder, mock_roster, mock_player): + async def test_validate_transaction_legal(self, builder, mock_player): """Test validation of a legal transaction.""" - with patch.object(builder, '_current_roster', mock_roster): - with patch.object(builder, '_roster_loaded', True): - # Add a move that keeps roster under limit (24 -> 25) - move = TransactionMove( - player=mock_player, - from_roster=RosterType.FREE_AGENCY, - to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team - ) - success, _ = await builder.add_move(move, check_pending_transactions=False) - assert success + # Add a move that keeps roster under limit (24 -> 25) + move = TransactionMove( + player=mock_player, + from_roster=RosterType.FREE_AGENCY, + to_roster=RosterType.MAJOR_LEAGUE, + to_team=builder.team, + ) + success, _ = await builder.add_move(move, check_pending_transactions=False) + assert success - validation = await builder.validate_transaction() + validation = await builder.validate_transaction() - assert validation.is_legal is True - assert validation.major_league_count == 25 # 24 + 1 - assert len(validation.errors) == 0 + assert validation.is_legal is True + assert validation.major_league_count == 25 # 24 + 1 + assert len(validation.errors) == 0 @pytest.mark.asyncio - async def test_validate_transaction_over_limit(self, builder, mock_roster): + async def test_validate_transaction_over_limit(self, builder): """Test validation when transaction would exceed roster limit.""" - with patch.object(builder, '_current_roster', mock_roster): - with patch.object(builder, '_roster_loaded', True): - # Add 3 players to exceed limit (24 + 3 = 27 > 26) - for i in range(3): - player = Player( - id=3000 + i, - name=f'New Player {i}', - wara=1.0, - season=12, - pos_1='OF' - ) - move = TransactionMove( - player=player, - from_roster=RosterType.FREE_AGENCY, - to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team - ) - success, _ = await builder.add_move(move, check_pending_transactions=False) - assert success + # Add 3 players to exceed limit (24 + 3 = 27 > 26) + for i in range(3): + player = Player( + id=3000 + i, + name=f"New Player {i}", + wara=1.0, + season=12, + pos_1="OF", + ) + move = TransactionMove( + player=player, + from_roster=RosterType.FREE_AGENCY, + to_roster=RosterType.MAJOR_LEAGUE, + to_team=builder.team, + ) + success, _ = await builder.add_move(move, check_pending_transactions=False) + assert success + + validation = await builder.validate_transaction() + + assert validation.is_legal is False + assert validation.major_league_count == 27 # 24 + 3 + assert len(validation.errors) == 1 + assert "27 players (limit: 26)" in validation.errors[0] + assert len(validation.suggestions) == 1 + assert "Drop 1 ML player" in validation.suggestions[0] - validation = await builder.validate_transaction() - - assert validation.is_legal is False - assert validation.major_league_count == 27 # 24 + 3 - assert len(validation.errors) == 1 - assert "27 players (limit: 26)" in validation.errors[0] - assert len(validation.suggestions) == 1 - assert "Drop 1 ML player" in validation.suggestions[0] - @pytest.mark.asyncio - async def test_validate_transaction_empty(self, builder, mock_roster): + async def test_validate_transaction_empty(self, builder): """Test validation of empty transaction.""" - with patch.object(builder, '_current_roster', mock_roster): - with patch.object(builder, '_roster_loaded', True): - validation = await builder.validate_transaction() - - assert validation.is_legal is True # Empty transaction is legal - assert validation.major_league_count == 24 # No changes - assert len(validation.suggestions) == 1 - assert "Add player moves" in validation.suggestions[0] + validation = await builder.validate_transaction() + + assert validation.is_legal is True # Empty transaction is legal + assert validation.major_league_count == 24 # No changes + assert len(validation.suggestions) == 1 + assert "Add player moves" in validation.suggestions[0] @pytest.mark.asyncio async def test_validate_transaction_over_swar_cap(self, mock_roster): @@ -416,39 +432,43 @@ class TestTransactionBuilder: # Create a team with a low salary cap (30.0) low_cap_team = Team( id=499, - abbrev='WV', - sname='Black Bears', - lname='West Virginia Black Bears', + abbrev="WV", + sname="Black Bears", + lname="West Virginia Black Bears", season=12, - salary_cap=30.0 # Low cap - roster has 24 * 1.5 = 36.0 sWAR + salary_cap=30.0, # Low cap - roster has 24 * 1.5 = 36.0 sWAR ) - builder = TransactionBuilder(low_cap_team, user_id=12345) + mock_svc = AsyncMock() + mock_svc.get_current_roster.return_value = mock_roster + builder = TransactionBuilder(low_cap_team, user_id=12345, roster_svc=mock_svc) - with patch.object(builder, '_current_roster', mock_roster): - with patch.object(builder, '_roster_loaded', True): - validation = await builder.validate_transaction() + validation = await builder.validate_transaction() - # Should fail due to sWAR cap (36.0 > 30.0) - assert validation.is_legal is False - assert validation.major_league_swar == 36.0 # 24 players * 1.5 wara - assert validation.major_league_swar_cap == 30.0 - assert any("sWAR would be 36.00 (cap: 30.0)" in error for error in validation.errors) - assert any("Remove 6.00 sWAR" in suggestion for suggestion in validation.suggestions) + # Should fail due to sWAR cap (36.0 > 30.0) + assert validation.is_legal is False + assert validation.major_league_swar == 36.0 # 24 players * 1.5 wara + assert validation.major_league_swar_cap == 30.0 + assert any( + "sWAR would be 36.00 (cap: 30.0)" in error for error in validation.errors + ) + assert any( + "Remove 6.00 sWAR" in suggestion for suggestion in validation.suggestions + ) @pytest.mark.asyncio async def test_validate_transaction_under_swar_cap(self, mock_team, mock_roster): """Test validation passes when sWAR is under team cap.""" # mock_team has salary_cap=50.0, roster has 36.0 sWAR - builder = TransactionBuilder(mock_team, user_id=12345) + mock_svc = AsyncMock() + mock_svc.get_current_roster.return_value = mock_roster + builder = TransactionBuilder(mock_team, user_id=12345, roster_svc=mock_svc) - with patch.object(builder, '_current_roster', mock_roster): - with patch.object(builder, '_roster_loaded', True): - validation = await builder.validate_transaction() + validation = await builder.validate_transaction() - # Should pass - 36.0 < 50.0 - assert validation.is_legal is True - assert validation.major_league_swar == 36.0 - assert validation.major_league_swar_cap == 50.0 + # Should pass - 36.0 < 50.0 + assert validation.is_legal is True + assert validation.major_league_swar == 36.0 + assert validation.major_league_swar_cap == 50.0 @pytest.mark.asyncio async def test_swar_status_display_over_cap(self): @@ -462,7 +482,7 @@ class TestTransactionBuilder: suggestions=[], major_league_swar=35.5, minor_league_swar=3.0, - major_league_swar_cap=33.5 + major_league_swar_cap=33.5, ) assert "❌" in validation.major_league_swar_status assert "35.50/33.5" in validation.major_league_swar_status @@ -480,7 +500,7 @@ class TestTransactionBuilder: suggestions=[], major_league_swar=31.0, minor_league_swar=3.0, - major_league_swar_cap=33.5 + major_league_swar_cap=33.5, ) assert "✅" in validation.major_league_swar_status assert "31.00/33.5" in validation.major_league_swar_status @@ -491,113 +511,117 @@ class TestTransactionBuilder: """Test submitting empty transaction fails.""" with pytest.raises(ValueError, match="Cannot submit empty transaction"): await builder.submit_transaction(week=11) - + @pytest.mark.asyncio - async def test_submit_transaction_illegal(self, builder, mock_roster): + async def test_submit_transaction_illegal(self, builder): """Test submitting illegal transaction fails.""" - with patch.object(builder, '_current_roster', mock_roster): - with patch.object(builder, '_roster_loaded', True): - # Add moves that exceed limit - for i in range(3): # 24 + 3 = 27 > 25 - player = Player( - id=4000 + i, - name=f'Illegal Player {i}', - wara=1.5, - season=12, - pos_1='OF' - ) - move = TransactionMove( - player=player, - from_roster=RosterType.FREE_AGENCY, - to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team - ) - success, _ = await builder.add_move(move, check_pending_transactions=False) - assert success + # Add moves that exceed limit + for i in range(3): # 24 + 3 = 27 > 25 + player = Player( + id=4000 + i, + name=f"Illegal Player {i}", + wara=1.5, + season=12, + pos_1="OF", + ) + move = TransactionMove( + player=player, + from_roster=RosterType.FREE_AGENCY, + to_roster=RosterType.MAJOR_LEAGUE, + to_team=builder.team, + ) + success, _ = await builder.add_move(move, check_pending_transactions=False) + assert success - with pytest.raises(ValueError, match="Cannot submit illegal transaction"): - await builder.submit_transaction(week=11) + with pytest.raises(ValueError, match="Cannot submit illegal transaction"): + await builder.submit_transaction(week=11) @pytest.mark.asyncio - async def test_submit_transaction_success(self, builder, mock_roster, mock_player): + async def test_submit_transaction_success(self, builder, mock_player): """Test successful transaction submission.""" - with patch.object(builder, '_current_roster', mock_roster): - with patch.object(builder, '_roster_loaded', True): - # Add a legal move - move = TransactionMove( - player=mock_player, - from_roster=RosterType.FREE_AGENCY, - to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team - ) - success, _ = await builder.add_move(move, check_pending_transactions=False) - assert success + # Add a legal move + move = TransactionMove( + player=mock_player, + from_roster=RosterType.FREE_AGENCY, + to_roster=RosterType.MAJOR_LEAGUE, + to_team=builder.team, + ) + success, _ = await builder.add_move(move, check_pending_transactions=False) + assert success - transactions = await builder.submit_transaction(week=11) + transactions = await builder.submit_transaction(week=11) - assert len(transactions) == 1 - transaction = transactions[0] - assert isinstance(transaction, Transaction) - assert transaction.week == 11 - assert transaction.season == 12 - assert transaction.player == mock_player - assert transaction.newteam == builder.team - assert "Season-012-Week-11-" in transaction.moveid + assert len(transactions) == 1 + transaction = transactions[0] + assert isinstance(transaction, Transaction) + assert transaction.week == 11 + assert transaction.season == 12 + assert transaction.player == mock_player + assert transaction.newteam == builder.team + assert "Season-012-Week-11-" in transaction.moveid @pytest.mark.asyncio - async def test_submit_complex_transaction(self, builder, mock_roster): + async def test_submit_complex_transaction(self, builder): """Test submitting transaction with multiple moves.""" - with patch.object(builder, '_current_roster', mock_roster): - with patch.object(builder, '_roster_loaded', True): - # Add one player and drop one player (net zero) - add_player = Player(id=5001, name='Add Player', wara=2.0, season=12, pos_1='OF') - drop_player = Player(id=5002, name='Drop Player', wara=1.0, season=12, pos_1='OF') + # Add one player and drop one player (net zero) + add_player = Player(id=5001, name="Add Player", wara=2.0, season=12, pos_1="OF") + drop_player = Player( + id=5002, name="Drop Player", wara=1.0, season=12, pos_1="OF" + ) - add_move = TransactionMove( - player=add_player, - from_roster=RosterType.FREE_AGENCY, - to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team - ) + add_move = TransactionMove( + player=add_player, + from_roster=RosterType.FREE_AGENCY, + to_roster=RosterType.MAJOR_LEAGUE, + to_team=builder.team, + ) - drop_move = TransactionMove( - player=drop_player, - from_roster=RosterType.MAJOR_LEAGUE, - to_roster=RosterType.FREE_AGENCY, - from_team=builder.team - ) + drop_move = TransactionMove( + player=drop_player, + from_roster=RosterType.MAJOR_LEAGUE, + to_roster=RosterType.FREE_AGENCY, + from_team=builder.team, + ) - success1, _ = await builder.add_move(add_move, check_pending_transactions=False) - success2, _ = await builder.add_move(drop_move, check_pending_transactions=False) - assert success1 and success2 + success1, _ = await builder.add_move(add_move, check_pending_transactions=False) + success2, _ = await builder.add_move( + drop_move, check_pending_transactions=False + ) + assert success1 and success2 - transactions = await builder.submit_transaction(week=11) + transactions = await builder.submit_transaction(week=11) - assert len(transactions) == 2 - # Both transactions should have the same move_id - assert transactions[0].moveid == transactions[1].moveid + assert len(transactions) == 2 + # Both transactions should have the same move_id + assert transactions[0].moveid == transactions[1].moveid class TestTransactionMove: """Test TransactionMove dataclass functionality.""" - + @pytest.fixture def mock_player(self): """Create a mock player.""" - return Player(id=123, name='Test Player', wara=2.0, season=12, pos_1='OF') - + return Player(id=123, name="Test Player", wara=2.0, season=12, pos_1="OF") + @pytest.fixture def mock_team(self): """Create a mock team.""" - return Team(id=499, abbrev='WV', sname='Black Bears', lname='West Virginia Black Bears', season=12) - + return Team( + id=499, + abbrev="WV", + sname="Black Bears", + lname="West Virginia Black Bears", + season=12, + ) + def test_add_move_description(self, mock_player, mock_team): """Test ADD move description.""" move = TransactionMove( player=mock_player, from_roster=RosterType.FREE_AGENCY, to_roster=RosterType.MAJOR_LEAGUE, - to_team=mock_team + to_team=mock_team, ) expected = "➕ Test Player: FA → WV (ML)" @@ -609,7 +633,7 @@ class TestTransactionMove: player=mock_player, from_roster=RosterType.MAJOR_LEAGUE, to_roster=RosterType.FREE_AGENCY, - from_team=mock_team + from_team=mock_team, ) expected = "➖ Test Player: WV (ML) → FA" @@ -622,7 +646,7 @@ class TestTransactionMove: from_roster=RosterType.MINOR_LEAGUE, to_roster=RosterType.MAJOR_LEAGUE, from_team=mock_team, - to_team=mock_team + to_team=mock_team, ) expected = "⬆️ Test Player: WV (MiL) → WV (ML)" @@ -635,7 +659,7 @@ class TestTransactionMove: from_roster=RosterType.MAJOR_LEAGUE, to_roster=RosterType.MINOR_LEAGUE, from_team=mock_team, - to_team=mock_team + to_team=mock_team, ) expected = "⬇️ Test Player: WV (ML) → WV (MiL)" @@ -644,7 +668,7 @@ class TestTransactionMove: class TestRosterValidationResult: """Test RosterValidationResult functionality.""" - + def test_major_league_status_over_limit(self): """Test status when over major league limit.""" result = RosterValidationResult( @@ -653,12 +677,12 @@ class TestRosterValidationResult: minor_league_count=6, warnings=[], errors=[], - suggestions=[] + suggestions=[], ) expected = "❌ Major League: 27/26 (Over limit!)" assert result.major_league_status == expected - + def test_major_league_status_at_limit(self): """Test status when at major league limit.""" result = RosterValidationResult( @@ -667,12 +691,12 @@ class TestRosterValidationResult: minor_league_count=6, warnings=[], errors=[], - suggestions=[] + suggestions=[], ) expected = "✅ Major League: 26/26 (Legal)" assert result.major_league_status == expected - + def test_major_league_status_under_limit(self): """Test status when under major league limit.""" result = RosterValidationResult( @@ -681,12 +705,12 @@ class TestRosterValidationResult: minor_league_count=6, warnings=[], errors=[], - suggestions=[] + suggestions=[], ) expected = "✅ Major League: 23/26 (Legal)" assert result.major_league_status == expected - + def test_minor_league_status(self): """Test minor league status with limit.""" result = RosterValidationResult( @@ -695,7 +719,7 @@ class TestRosterValidationResult: minor_league_count=7, warnings=[], errors=[], - suggestions=[] + suggestions=[], ) expected = "❌ Minor League: 7/6 (Over limit!)" @@ -704,39 +728,57 @@ class TestRosterValidationResult: class TestTransactionBuilderGlobalFunctions: """Test global transaction builder functions.""" - + def test_get_transaction_builder_new(self): """Test getting new transaction builder.""" - team = Team(id=499, abbrev='WV', sname='Black Bears', lname='West Virginia Black Bears', season=12) - + team = Team( + id=499, + abbrev="WV", + sname="Black Bears", + lname="West Virginia Black Bears", + season=12, + ) + builder = get_transaction_builder(user_id=123, team=team) - + assert isinstance(builder, TransactionBuilder) assert builder.user_id == 123 assert builder.team == team - + def test_get_transaction_builder_existing(self): """Test getting existing transaction builder.""" - team = Team(id=499, abbrev='WV', sname='Black Bears', lname='West Virginia Black Bears', season=12) - + team = Team( + id=499, + abbrev="WV", + sname="Black Bears", + lname="West Virginia Black Bears", + season=12, + ) + builder1 = get_transaction_builder(user_id=123, team=team) builder2 = get_transaction_builder(user_id=123, team=team) - + assert builder1 is builder2 # Should return same instance - + def test_clear_transaction_builder(self): """Test clearing transaction builder.""" - team = Team(id=499, abbrev='WV', sname='Black Bears', lname='West Virginia Black Bears', season=12) - + team = Team( + id=499, + abbrev="WV", + sname="Black Bears", + lname="West Virginia Black Bears", + season=12, + ) + builder = get_transaction_builder(user_id=123, team=team) assert builder is not None - + clear_transaction_builder(user_id=123) - + # Getting builder again should create new instance new_builder = get_transaction_builder(user_id=123, team=team) assert new_builder is not builder - + def test_clear_nonexistent_builder(self): """Test clearing non-existent builder doesn't error.""" # Should not raise any exception @@ -756,22 +798,16 @@ class TestPendingTransactionValidation: """Create a mock team for testing.""" return Team( id=499, - abbrev='WV', - sname='Black Bears', - lname='West Virginia Black Bears', - season=12 + abbrev="WV", + sname="Black Bears", + lname="West Virginia Black Bears", + season=12, ) @pytest.fixture def mock_player(self): """Create a mock player for testing.""" - return Player( - id=12472, - name='Test Player', - wara=2.5, - season=12, - pos_1='OF' - ) + return Player(id=12472, name="Test Player", wara=2.5, season=12, pos_1="OF") @pytest.fixture def builder(self, mock_team): @@ -779,7 +815,9 @@ class TestPendingTransactionValidation: return TransactionBuilder(mock_team, user_id=123, season=12) @pytest.mark.asyncio - async def test_add_move_player_in_pending_transaction_fails(self, builder, mock_player): + async def test_add_move_player_in_pending_transaction_fails( + self, builder, mock_player + ): """ Test that adding a player who is already in a pending transaction fails. @@ -790,11 +828,15 @@ class TestPendingTransactionValidation: player=mock_player, from_roster=RosterType.FREE_AGENCY, to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team + to_team=builder.team, ) - with patch('services.transaction_builder.transaction_service') as mock_tx_service: - with patch('services.transaction_builder.league_service') as mock_league_service: + with patch( + "services.transaction_builder.transaction_service" + ) as mock_tx_service: + with patch( + "services.transaction_builder.league_service" + ) as mock_league_service: # Mock that player IS in a pending transaction (claimed by LAA) mock_tx_service.is_player_in_pending_transaction = AsyncMock( return_value=(True, "LAA") @@ -813,7 +855,9 @@ class TestPendingTransactionValidation: assert builder.move_count == 0 @pytest.mark.asyncio - async def test_add_move_player_not_in_pending_transaction_succeeds(self, builder, mock_player): + async def test_add_move_player_not_in_pending_transaction_succeeds( + self, builder, mock_player + ): """ Test that adding a player who is NOT in a pending transaction succeeds. @@ -824,11 +868,15 @@ class TestPendingTransactionValidation: player=mock_player, from_roster=RosterType.FREE_AGENCY, to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team + to_team=builder.team, ) - with patch('services.transaction_builder.transaction_service') as mock_tx_service: - with patch('services.transaction_builder.league_service') as mock_league_service: + with patch( + "services.transaction_builder.transaction_service" + ) as mock_tx_service: + with patch( + "services.transaction_builder.league_service" + ) as mock_league_service: # Mock that player is NOT in a pending transaction mock_tx_service.is_player_in_pending_transaction = AsyncMock( return_value=(False, None) @@ -856,11 +904,13 @@ class TestPendingTransactionValidation: player=mock_player, from_roster=RosterType.FREE_AGENCY, to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team + to_team=builder.team, ) # Even if the service would return True, the check should be skipped - with patch('services.transaction_builder.transaction_service') as mock_tx_service: + with patch( + "services.transaction_builder.transaction_service" + ) as mock_tx_service: # This mock should NOT be called when check_pending_transactions=False mock_tx_service.is_player_in_pending_transaction = AsyncMock( return_value=(True, "LAA") @@ -888,10 +938,12 @@ class TestPendingTransactionValidation: player=mock_player, from_roster=RosterType.FREE_AGENCY, to_roster=RosterType.MAJOR_LEAGUE, - to_team=builder.team + to_team=builder.team, ) - with patch('services.transaction_builder.transaction_service') as mock_tx_service: + with patch( + "services.transaction_builder.transaction_service" + ) as mock_tx_service: mock_tx_service.is_player_in_pending_transaction = AsyncMock( return_value=(False, None) ) @@ -902,4 +954,4 @@ class TestPendingTransactionValidation: # Verify the check was called with the explicit week mock_tx_service.is_player_in_pending_transaction.assert_called_once() call_args = mock_tx_service.is_player_in_pending_transaction.call_args - assert call_args.kwargs['week'] == 15 \ No newline at end of file + assert call_args.kwargs["week"] == 15