From 3cbc904478b7bcd252dcff8312fe1d60b2abcd4d Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Sun, 21 Dec 2025 17:13:43 -0600 Subject: [PATCH] Add pending transaction validation for /dropadd command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevents players who are already claimed in another team's pending transaction (frozen=false, cancelled=false) from being added to a new transaction for the same week. Changes: - Add is_player_in_pending_transaction() to TransactionService - Make TransactionBuilder.add_move() async with validation - Add check_pending_transactions flag (default True for /dropadd) - Skip validation for /ilmove and trades (check_pending_transactions=False) - Add tests/conftest.py for proper test isolation - Add 4 new tests for pending transaction validation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- commands/transactions/dropadd.py | 2 +- commands/transactions/ilmove.py | 4 +- services/trade_builder.py | 8 +- services/transaction_builder.py | 35 ++- services/transaction_service.py | 52 +++- tests/conftest.py | 53 +++++ tests/test_commands_dropadd.py | 4 +- tests/test_dropadd_integration.py | 16 +- tests/test_services_trade_builder.py | 5 +- tests/test_services_transaction_builder.py | 264 +++++++++++++++++---- 10 files changed, 378 insertions(+), 65 deletions(-) create mode 100644 tests/conftest.py diff --git a/commands/transactions/dropadd.py b/commands/transactions/dropadd.py index 23b8f2a..4fa5df9 100644 --- a/commands/transactions/dropadd.py +++ b/commands/transactions/dropadd.py @@ -234,7 +234,7 @@ class DropAddCommands(commands.Cog): to_team=None if to_roster == RosterType.FREE_AGENCY else builder.team ) - success, error_message = builder.add_move(move) + success, error_message = await builder.add_move(move) if not success: self.logger.warning(f"Failed to add quick move: {error_message}") return False, error_message diff --git a/commands/transactions/ilmove.py b/commands/transactions/ilmove.py index 29cf211..f801906 100644 --- a/commands/transactions/ilmove.py +++ b/commands/transactions/ilmove.py @@ -29,6 +29,7 @@ from services.transaction_builder import ( ) from services.player_service import player_service from services.team_service import team_service +from services.league_service import league_service from views.transaction_embed import TransactionEmbedView, create_transaction_embed @@ -255,7 +256,8 @@ class ILMoveCommands(commands.Cog): to_team=to_team ) - success, error_message = builder.add_move(move) + # For /ilmove, skip pending transaction check (immediate moves don't need it) + success, error_message = await builder.add_move(move, check_pending_transactions=False) if not success: self.logger.warning(f"Failed to add quick move: {error_message}") return False, error_message diff --git a/services/trade_builder.py b/services/trade_builder.py index a129928..6550ffe 100644 --- a/services/trade_builder.py +++ b/services/trade_builder.py @@ -318,14 +318,15 @@ class TradeBuilder: ) # Add moves to respective builders - from_success, from_error = from_builder.add_move(from_move) + # 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) 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 = to_builder.add_move(to_move) + 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) @@ -383,7 +384,8 @@ class TradeBuilder: to_team=team ) - success, error = builder.add_move(trans_move) + # Skip pending transaction check for trade supplementary moves + success, error = await builder.add_move(trans_move, check_pending_transactions=False) if not success: participant.supplementary_moves.remove(supp_move) return False, error diff --git a/services/transaction_builder.py b/services/transaction_builder.py index af0f6e1..7d7d1f4 100644 --- a/services/transaction_builder.py +++ b/services/transaction_builder.py @@ -194,17 +194,25 @@ class TransactionBuilder: self._existing_transactions = [] self._existing_transactions_loaded = True - def add_move(self, move: TransactionMove) -> tuple[bool, str]: + async def add_move( + self, + move: TransactionMove, + next_week: Optional[int] = None, + check_pending_transactions: bool = True + ) -> tuple[bool, str]: """ Add a move to the transaction. Args: move: TransactionMove to add + next_week: Week number for pending transaction check (optional, will be fetched if not provided) + check_pending_transactions: Whether to check if player is in another team's pending + transaction. Set to True for /dropadd (scheduled moves), False for /ilmove (immediate). Returns: Tuple of (success: bool, error_message: str). If success is True, error_message is empty. """ - # Check if player is already in a move + # Check if player is already in a move in this transaction builder existing_move = self.get_move_for_player(move.player.id) if existing_move: error_msg = f"Player {move.player.name} already has a move in this transaction" @@ -219,6 +227,29 @@ class TransactionBuilder: logger.warning(error_msg) return False, error_msg + # Check if player is already in another team's pending transaction for next week + # This prevents duplicate claims that would need to be resolved at freeze time + # Only applies to /dropadd (scheduled moves), not /ilmove (immediate moves) + if check_pending_transactions: + if next_week is None: + try: + 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 get current week for pending transaction check: {e}") + next_week = 1 + + is_pending, claiming_team = await transaction_service.is_player_in_pending_transaction( + player_id=move.player.id, + week=next_week, + season=self.season + ) + + if is_pending: + error_msg = f"{move.player.name} is already in a pending transaction for week {next_week} (claimed by {claiming_team})" + logger.warning(error_msg) + return False, error_msg + self.moves.append(move) logger.info(f"Added move: {move.description}") return True, "" diff --git a/services/transaction_service.py b/services/transaction_service.py index d6777ba..ff2ba72 100644 --- a/services/transaction_service.py +++ b/services/transaction_service.py @@ -434,11 +434,61 @@ class TransactionService(BaseService[Transaction]): logger.debug(f"Found {len(result)} potentially contested transactions for week {week}") return result - + except Exception as e: logger.error(f"Error getting contested transactions: {e}") return [] + async def is_player_in_pending_transaction( + self, + player_id: int, + week: int, + season: int + ) -> tuple[bool, Optional[str]]: + """ + Check if a player is already in a pending transaction for a specific week. + + This checks ALL teams' pending transactions (frozen=false, cancelled=false) + to prevent duplicate claims on the same player. + + Args: + player_id: Player ID to check + week: Week number to check + season: Season number + + Returns: + Tuple of (is_in_pending_transaction, claiming_team_abbrev or None) + """ + try: + # Get all pending transactions for the week (all teams) + params = [ + ('season', str(season)), + ('week', str(week)), + ('cancelled', 'false'), + ('frozen', 'false') + ] + + transactions = await self.get_all_items(params=params) + + # Check if the player is in any of these transactions + for transaction in transactions: + if transaction.player and transaction.player.id == player_id: + # Found the player in a pending transaction + claiming_team = transaction.newteam.abbrev if transaction.newteam else "Unknown" + logger.info( + f"Player {player_id} already in pending transaction for week {week} " + f"(claimed by {claiming_team})" + ) + return True, claiming_team + + return False, None + + except Exception as e: + logger.error(f"Error checking pending transactions for player {player_id}: {e}") + # On error, allow the transaction (fail open) but log the issue + # The freeze task will still catch duplicates if they occur + return False, None + # Global service instance transaction_service = TransactionService() \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..3083dd0 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,53 @@ +""" +Pytest configuration and fixtures for Discord Bot v2.0 tests. + +This file provides test isolation and shared fixtures. +""" +import asyncio +import os +import pytest + +# Ensure environment is set up before any imports happen +# This is critical for tests that check GUILD_ID +os.environ.setdefault("GUILD_ID", "669356687294988350") +os.environ.setdefault("TESTING", "true") + + +@pytest.fixture(autouse=True) +def reset_singleton_state(): + """ + Reset any singleton/global state between tests. + + This prevents test pollution from global state in services. + """ + # Import after test function starts to ensure clean state + yield # Run test + + # Cleanup after test + # Reset transaction builder caches + try: + from services.transaction_builder import _transaction_builders + _transaction_builders.clear() + except ImportError: + pass + + try: + from services.trade_builder import _trade_builders, _team_to_trade_key + _trade_builders.clear() + _team_to_trade_key.clear() + except ImportError: + pass + + # Reset config singleton to ensure clean state + try: + from config import _config + import config as cfg + cfg._config = None + except (ImportError, AttributeError): + pass + + +@pytest.fixture(scope="session") +def event_loop_policy(): + """Use default event loop policy.""" + return asyncio.DefaultEventLoopPolicy() diff --git a/tests/test_commands_dropadd.py b/tests/test_commands_dropadd.py index 798494a..1a747a7 100644 --- a/tests/test_commands_dropadd.py +++ b/tests/test_commands_dropadd.py @@ -178,7 +178,7 @@ class TestDropAddCommands: """Test successful quick move addition.""" mock_builder = MagicMock() mock_builder.team = mock_team - mock_builder.add_move.return_value = (True, "") + mock_builder.add_move = AsyncMock(return_value=(True, "")) # Now async mock_builder.load_roster_data = AsyncMock() mock_builder._current_roster = MagicMock() mock_builder._current_roster.active_players = [] @@ -262,7 +262,7 @@ class TestDropAddCommands: mock_builder = MagicMock() mock_builder.team = mock_team - mock_builder.add_move.return_value = (True, "") + mock_builder.add_move = AsyncMock(return_value=(True, "")) # Now async mock_builder.load_roster_data = AsyncMock() mock_builder._current_roster = MagicMock() mock_builder._current_roster.active_players = [] diff --git a/tests/test_dropadd_integration.py b/tests/test_dropadd_integration.py index 365520a..c4cfd0e 100644 --- a/tests/test_dropadd_integration.py +++ b/tests/test_dropadd_integration.py @@ -182,6 +182,8 @@ class TestDropAddIntegration: mock_player_service.search_players = AsyncMock(return_value=[mock_players[0]]) # Mike Trout mock_roster_service.get_current_roster = AsyncMock(return_value=mock_roster) mock_tx_service.get_team_transactions = AsyncMock(return_value=[]) + # Mock the new pending transaction check + mock_tx_service.is_player_in_pending_transaction = AsyncMock(return_value=(False, None)) # Execute /dropadd command with quick move await commands_cog.dropadd.callback(commands_cog, @@ -246,8 +248,8 @@ class TestDropAddIntegration: from_team=mock_team ) - builder.add_move(add_move) - builder.add_move(drop_move) + await builder.add_move(add_move, check_pending_transactions=False) + await builder.add_move(drop_move, check_pending_transactions=False) # Verify multi-move transaction assert builder.move_count == 2 @@ -278,7 +280,7 @@ class TestDropAddIntegration: to_roster=RosterType.MAJOR_LEAGUE, to_team=mock_team ) - builder.add_move(move) + await builder.add_move(move, check_pending_transactions=False) # Test submission transactions = await builder.submit_transaction(week=11) @@ -329,7 +331,7 @@ class TestDropAddIntegration: to_roster=RosterType.MAJOR_LEAGUE, to_team=mock_team ) - builder.add_move(move) + await builder.add_move(move, check_pending_transactions=False) # Submit transactions first to get move IDs transactions = await builder.submit_transaction(week=mock_current_state.week + 1) @@ -338,7 +340,7 @@ class TestDropAddIntegration: # Reset the builder and add move again for modal test clear_transaction_builder(mock_interaction.user.id) builder = get_transaction_builder(mock_interaction.user.id, mock_team) - builder.add_move(move) + await builder.add_move(move, check_pending_transactions=False) # Create the modal modal = SubmitConfirmationModal(builder) @@ -445,7 +447,7 @@ class TestDropAddIntegration: to_roster=RosterType.MAJOR_LEAGUE, to_team=mock_team ) - builder.add_move(move) + await builder.add_move(move, check_pending_transactions=False) # Test validation validation = await builder.validate_transaction() @@ -483,7 +485,7 @@ class TestDropAddIntegration: to_roster=RosterType.MAJOR_LEAGUE, to_team=mock_team ) - builder1.add_move(move) + await builder1.add_move(move, check_pending_transactions=False) assert builder1.move_count == 1 # Second command call should get same builder diff --git a/tests/test_services_trade_builder.py b/tests/test_services_trade_builder.py index 819e177..cd60564 100644 --- a/tests/test_services_trade_builder.py +++ b/tests/test_services_trade_builder.py @@ -411,8 +411,9 @@ class TestTradeBuilder: mock_builder2.validate_transaction = AsyncMock(return_value=valid_result) # Configure add_move methods to return expected tuple (success, error_message) - mock_builder1.add_move.return_value = (True, "") - mock_builder2.add_move.return_value = (True, "") + # add_move is now async, so use AsyncMock + mock_builder1.add_move = AsyncMock(return_value=(True, "")) + mock_builder2.add_move = AsyncMock(return_value=(True, "")) def get_builder_side_effect(team): if team.id == self.team1.id: diff --git a/tests/test_services_transaction_builder.py b/tests/test_services_transaction_builder.py index 9a7e4b2..76abd7d 100644 --- a/tests/test_services_transaction_builder.py +++ b/tests/test_services_transaction_builder.py @@ -124,7 +124,8 @@ class TestTransactionBuilder: assert builder.move_count == 0 assert len(builder.moves) == 0 - def test_add_move_success(self, builder, mock_player): + @pytest.mark.asyncio + async def test_add_move_success(self, builder, mock_player): """Test successfully adding a move.""" move = TransactionMove( player=mock_player, @@ -133,15 +134,17 @@ class TestTransactionBuilder: to_team=builder.team ) - success, error_message = builder.add_move(move) + # Skip pending transaction check for unit tests + success, error_message = await builder.add_move(move, check_pending_transactions=False) assert success is True assert error_message == "" assert builder.move_count == 1 assert builder.is_empty is False assert move in builder.moves - - def test_add_duplicate_move_fails(self, builder, mock_player): + + @pytest.mark.asyncio + async def test_add_duplicate_move_fails(self, builder, mock_player): """Test that adding duplicate moves for same player fails.""" move1 = TransactionMove( player=mock_player, @@ -157,8 +160,8 @@ class TestTransactionBuilder: from_team=builder.team ) - success1, error_message1 = builder.add_move(move1) - success2, error_message2 = builder.add_move(move2) + 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 == "" @@ -166,7 +169,8 @@ class TestTransactionBuilder: assert "already has a move" in error_message2 assert builder.move_count == 1 - def test_add_move_same_team_same_roster_fails(self, builder, mock_player): + @pytest.mark.asyncio + async def test_add_move_same_team_same_roster_fails(self, builder, mock_player): """Test that adding a move where from_team, to_team, from_roster, and to_roster are all the same fails.""" move = TransactionMove( player=mock_player, @@ -176,14 +180,15 @@ class TestTransactionBuilder: to_team=builder.team # Same team - should fail when roster is also same ) - success, error_message = builder.add_move(move) + success, error_message = await builder.add_move(move, check_pending_transactions=False) assert success is False assert "already in that location" in error_message assert builder.move_count == 0 assert builder.is_empty is True - def test_add_move_same_team_different_roster_succeeds(self, builder, mock_player): + @pytest.mark.asyncio + 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, @@ -193,14 +198,15 @@ class TestTransactionBuilder: to_team=builder.team # Same team - should succeed when rosters differ ) - success, error_message = builder.add_move(move) + success, error_message = await builder.add_move(move, check_pending_transactions=False) assert success is True assert error_message == "" assert builder.move_count == 1 assert builder.is_empty is False - def test_add_move_different_teams_succeeds(self, builder, mock_player): + @pytest.mark.asyncio + 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, @@ -218,14 +224,15 @@ class TestTransactionBuilder: to_team=builder.team ) - success, error_message = builder.add_move(move) + success, error_message = await builder.add_move(move, check_pending_transactions=False) assert success is True assert error_message == "" assert builder.move_count == 1 assert builder.is_empty is False - def test_add_move_none_teams_succeeds(self, builder, mock_player): + @pytest.mark.asyncio + async def test_add_move_none_teams_succeeds(self, builder, mock_player): """Test that adding a move where one or both teams are None succeeds.""" # From FA to team (from_team=None) move1 = TransactionMove( @@ -236,7 +243,7 @@ class TestTransactionBuilder: to_team=builder.team ) - success1, error_message1 = builder.add_move(move1) + success1, error_message1 = await builder.add_move(move1, check_pending_transactions=False) assert success1 is True assert error_message1 == "" @@ -260,11 +267,12 @@ class TestTransactionBuilder: to_team=None ) - success2, error_message2 = builder.add_move(move2) + success2, error_message2 = await builder.add_move(move2, check_pending_transactions=False) assert success2 is True assert error_message2 == "" - - def test_remove_move_success(self, builder, mock_player): + + @pytest.mark.asyncio + async def test_remove_move_success(self, builder, mock_player): """Test successfully removing a move.""" move = TransactionMove( player=mock_player, @@ -273,7 +281,7 @@ class TestTransactionBuilder: to_team=builder.team ) - success, _ = builder.add_move(move) + success, _ = await builder.add_move(move, check_pending_transactions=False) assert success assert builder.move_count == 1 @@ -290,7 +298,8 @@ class TestTransactionBuilder: assert removed is False assert builder.move_count == 0 - def test_get_move_for_player(self, builder, mock_player): + @pytest.mark.asyncio + async def test_get_move_for_player(self, builder, mock_player): """Test getting move for a specific player.""" move = TransactionMove( player=mock_player, @@ -299,15 +308,16 @@ class TestTransactionBuilder: to_team=builder.team ) - builder.add_move(move) + await builder.add_move(move, check_pending_transactions=False) found_move = builder.get_move_for_player(mock_player.id) not_found = builder.get_move_for_player(99999) assert found_move == move assert not_found is None - - def test_clear_moves(self, builder, mock_player): + + @pytest.mark.asyncio + async def test_clear_moves(self, builder, mock_player): """Test clearing all moves.""" move = TransactionMove( player=mock_player, @@ -316,7 +326,7 @@ class TestTransactionBuilder: to_team=builder.team ) - success, _ = builder.add_move(move) + success, _ = await builder.add_move(move, check_pending_transactions=False) assert success assert builder.move_count == 1 @@ -324,18 +334,18 @@ class TestTransactionBuilder: assert builder.move_count == 0 assert builder.is_empty is True - + @pytest.mark.asyncio async def test_validate_transaction_no_roster(self, builder): """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() - + 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): """Test validation of a legal transaction.""" @@ -348,15 +358,15 @@ class TestTransactionBuilder: to_roster=RosterType.MAJOR_LEAGUE, to_team=builder.team ) - success, _ = builder.add_move(move) + success, _ = await builder.add_move(move, check_pending_transactions=False) assert success - + validation = await builder.validate_transaction() - + 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): """Test validation when transaction would exceed roster limit.""" @@ -377,9 +387,9 @@ class TestTransactionBuilder: to_roster=RosterType.MAJOR_LEAGUE, to_team=builder.team ) - success, _ = builder.add_move(move) + success, _ = await builder.add_move(move, check_pending_transactions=False) assert success - + validation = await builder.validate_transaction() assert validation.is_legal is False @@ -427,12 +437,12 @@ class TestTransactionBuilder: to_roster=RosterType.MAJOR_LEAGUE, to_team=builder.team ) - success, _ = builder.add_move(move) + 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) - + @pytest.mark.asyncio async def test_submit_transaction_success(self, builder, mock_roster, mock_player): """Test successful transaction submission.""" @@ -445,11 +455,11 @@ class TestTransactionBuilder: to_roster=RosterType.MAJOR_LEAGUE, to_team=builder.team ) - success, _ = builder.add_move(move) + success, _ = await builder.add_move(move, check_pending_transactions=False) assert success - + transactions = await builder.submit_transaction(week=11) - + assert len(transactions) == 1 transaction = transactions[0] assert isinstance(transaction, Transaction) @@ -458,7 +468,7 @@ class TestTransactionBuilder: 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): """Test submitting transaction with multiple moves.""" @@ -467,7 +477,7 @@ class TestTransactionBuilder: # 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, @@ -481,13 +491,13 @@ class TestTransactionBuilder: to_roster=RosterType.FREE_AGENCY, from_team=builder.team ) - - success1, _ = builder.add_move(add_move) - success2, _ = builder.add_move(drop_move) + + 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) - + assert len(transactions) == 2 # Both transactions should have the same move_id assert transactions[0].moveid == transactions[1].moveid @@ -655,4 +665,166 @@ class TestTransactionBuilderGlobalFunctions: def test_clear_nonexistent_builder(self): """Test clearing non-existent builder doesn't error.""" # Should not raise any exception - clear_transaction_builder(user_id=99999) \ No newline at end of file + clear_transaction_builder(user_id=99999) + + +class TestPendingTransactionValidation: + """ + Test pending transaction validation in add_move. + + This validates that players who are already in a pending transaction + for the next week cannot be added to another transaction. + """ + + @pytest.fixture + def mock_team(self): + """Create a mock team for testing.""" + return Team( + id=499, + 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' + ) + + @pytest.fixture + def builder(self, mock_team): + """Create a TransactionBuilder for testing.""" + 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): + """ + Test that adding a player who is already in a pending transaction fails. + + When a player is claimed by another team in a pending (unfrozen) transaction + for the next week, they should not be able to be added to another transaction. + """ + move = TransactionMove( + player=mock_player, + from_roster=RosterType.FREE_AGENCY, + to_roster=RosterType.MAJOR_LEAGUE, + 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: + # Mock that player IS in a pending transaction (claimed by LAA) + mock_tx_service.is_player_in_pending_transaction = AsyncMock( + return_value=(True, "LAA") + ) + # Mock current state to provide next week + mock_league_service.get_current_state = AsyncMock( + return_value=MagicMock(week=10) + ) + + success, error_message = await builder.add_move(move) + + assert success is False + assert "already in a pending transaction" in error_message + assert "week 11" in error_message + assert "LAA" in error_message + assert builder.move_count == 0 + + @pytest.mark.asyncio + 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. + + When a player is available (not claimed in any pending transaction), + they should be able to be added to the transaction builder. + """ + move = TransactionMove( + player=mock_player, + from_roster=RosterType.FREE_AGENCY, + to_roster=RosterType.MAJOR_LEAGUE, + 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: + # Mock that player is NOT in a pending transaction + mock_tx_service.is_player_in_pending_transaction = AsyncMock( + return_value=(False, None) + ) + # Mock current state to provide next week + mock_league_service.get_current_state = AsyncMock( + return_value=MagicMock(week=10) + ) + + success, error_message = await builder.add_move(move) + + assert success is True + assert error_message == "" + assert builder.move_count == 1 + + @pytest.mark.asyncio + async def test_add_move_skip_pending_check_with_flag(self, builder, mock_player): + """ + Test that check_pending_transactions=False skips the validation. + + This is used for IL moves and trade moves where the pending transaction + check should not apply. + """ + move = TransactionMove( + player=mock_player, + from_roster=RosterType.FREE_AGENCY, + to_roster=RosterType.MAJOR_LEAGUE, + 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: + # This mock should NOT be called when check_pending_transactions=False + mock_tx_service.is_player_in_pending_transaction = AsyncMock( + return_value=(True, "LAA") + ) + + success, error_message = await builder.add_move( + move, check_pending_transactions=False + ) + + assert success is True + assert error_message == "" + assert builder.move_count == 1 + # Verify the service method was NOT called + mock_tx_service.is_player_in_pending_transaction.assert_not_called() + + @pytest.mark.asyncio + async def test_add_move_with_explicit_next_week(self, builder, mock_player): + """ + Test that providing next_week parameter uses that value. + + The next_week parameter allows the caller to specify which week + to check for pending transactions. + """ + move = TransactionMove( + player=mock_player, + from_roster=RosterType.FREE_AGENCY, + to_roster=RosterType.MAJOR_LEAGUE, + to_team=builder.team + ) + + with patch('services.transaction_builder.transaction_service') as mock_tx_service: + mock_tx_service.is_player_in_pending_transaction = AsyncMock( + return_value=(False, None) + ) + + success, error_message = await builder.add_move(move, next_week=15) + + assert success is True + # 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