diff --git a/commands/transactions/dropadd.py b/commands/transactions/dropadd.py index be467af..622e12a 100644 --- a/commands/transactions/dropadd.py +++ b/commands/transactions/dropadd.py @@ -38,26 +38,69 @@ class DropAddCommands(commands.Cog): current: str ) -> List[app_commands.Choice[str]]: """ - Autocomplete for player names. - + Autocomplete for player names with team context prioritization. + Args: interaction: Discord interaction current: Current input from user - + Returns: - List of player name choices + List of player name choices (user's team players first) """ if len(current) < 2: return [] - - try: - # Search for players using the new dedicated search endpoint - players = await player_service.search_players(current, limit=25, season=SBA_CURRENT_SEASON) - # Format choices for Discord autocomplete - choices = [] + try: + # Get user's team for prioritization + user_team = None + try: + major_league_teams = await team_service.get_teams_by_owner( + interaction.user.id, + SBA_CURRENT_SEASON, + roster_type="ml" + ) + if major_league_teams: + user_team = major_league_teams[0] + except Exception: + # If we can't get user's team, continue without prioritization + pass + + # Search for players using the search endpoint + players = await player_service.search_players(current, limit=50, season=SBA_CURRENT_SEASON) + + # Separate players by team (user's team vs others) + user_team_players = [] + other_players = [] + for player in players: - # Format: "Player Name (POS - TEAM)" + # Check if player belongs to user's team (any roster section) + is_users_player = False + if user_team and hasattr(player, 'team') and player.team: + # Check if player is from user's major league team or has same base team + if (player.team.id == user_team.id or + (hasattr(player, 'team_id') and player.team_id == user_team.id)): + is_users_player = True + + if is_users_player: + user_team_players.append(player) + else: + other_players.append(player) + + # Format choices with team context + choices = [] + + # Add user's team players first (prioritized) + for player in user_team_players[:15]: # Limit user team players + team_info = f"{player.primary_position}" + if hasattr(player, 'team') and player.team: + team_info += f" - {player.team.abbrev}" + + choice_name = f"{player.name} ({team_info})" + choices.append(app_commands.Choice(name=choice_name, value=player.name)) + + # Add other players (remaining slots) + remaining_slots = 25 - len(choices) + for player in other_players[:remaining_slots]: team_info = f"{player.primary_position}" if hasattr(player, 'team') and player.team: team_info += f" - {player.team.abbrev}" @@ -66,7 +109,7 @@ class DropAddCommands(commands.Cog): choices.append(app_commands.Choice(name=choice_name, value=player.name)) return choices - + except Exception as e: self.logger.error(f"Error in player autocomplete: {e}") return [] @@ -76,7 +119,7 @@ class DropAddCommands(commands.Cog): description="Interactive transaction builder for player moves" ) @app_commands.describe( - player="Player name (optional - can add later)", + player="Player name (use autocomplete for best results)", destination="Where to move the player: Major League, Minor League, Injured List, or Free Agency" ) @app_commands.autocomplete(player=player_autocomplete) @@ -94,8 +137,8 @@ class DropAddCommands(commands.Cog): destination: Optional[str] = None ): """Interactive transaction builder for complex player moves.""" - await interaction.response.defer() - + await interaction.response.defer(ephemeral=True) + # Get user's major league team major_league_teams = await team_service.get_teams_by_owner( interaction.user.id, @@ -111,47 +154,74 @@ class DropAddCommands(commands.Cog): return team = major_league_teams[0] # Use first major league team - + # Get or create transaction builder builder = get_transaction_builder(interaction.user.id, team) - - # If player and destination provided, try to add the move immediately + + # Handle different scenarios based on builder state and parameters if player and destination: - success = await self._add_quick_move(builder, player, destination) + # User provided both parameters - try to add the move + success, error_message = await self._add_quick_move(builder, player, destination) + if success: - self.logger.info(f"Quick move added for {team.abbrev}: {player} ā†’ {destination}") + # Move added successfully - show updated transaction builder + embed = await create_transaction_embed(builder) + view = TransactionEmbedView(builder, interaction.user.id) + + success_msg = f"āœ… **Added {player} ā†’ {destination.upper()}**" + if builder.move_count > 1: + success_msg += f"\nšŸ“Š Transaction now has {builder.move_count} moves" + + await interaction.followup.send( + content=success_msg, + embed=embed, + view=view, + ephemeral=True + ) + self.logger.info(f"Move added for {team.abbrev}: {player} ā†’ {destination}") + else: - self.logger.warning(f"Failed to add quick move: {player} ā†’ {destination}") - - # Create and display interactive embed - embed = await create_transaction_embed(builder) - view = TransactionEmbedView(builder, interaction.user.id) - - await interaction.followup.send(embed=embed, view=view) + # Failed to add move - still show current transaction state + embed = await create_transaction_embed(builder) + view = TransactionEmbedView(builder, interaction.user.id) + + await interaction.followup.send( + content=f"āŒ **{error_message}**\n" + f"šŸ’” Try using autocomplete for player names", + embed=embed, + view=view, + ephemeral=True + ) + self.logger.warning(f"Failed to add move: {player} ā†’ {destination}: {error_message}") + else: + # No parameters or incomplete parameters - show current transaction state + embed = await create_transaction_embed(builder) + view = TransactionEmbedView(builder, interaction.user.id) + await interaction.followup.send(embed=embed, view=view, ephemeral=True) async def _add_quick_move( - self, - builder: TransactionBuilder, - player_name: str, + self, + builder: TransactionBuilder, + player_name: str, destination_str: str - ) -> bool: + ) -> tuple[bool, str]: """ Add a move quickly from command parameters by auto-determining the action. - + Args: builder: TransactionBuilder instance player_name: Name of player to move destination_str: Destination string (ml, mil, fa) - + Returns: - True if move was added successfully + Tuple of (success: bool, error_message: str) """ try: # Find player using the new search endpoint players = await player_service.search_players(player_name, limit=10, season=SBA_CURRENT_SEASON) if not players: self.logger.error(f"Player not found: {player_name}") - return False + return False, f"Player '{player_name}' not found" # Use exact match if available, otherwise first result player = None @@ -159,9 +229,20 @@ class DropAddCommands(commands.Cog): if p.name.lower() == player_name.lower(): player = p break - + if not player: player = players[0] # Use first match + + # Check if player belongs to another team (not user's team and not Free Agency) + if player.team and hasattr(player.team, 'abbrev'): + # Player belongs to another team if: + # 1. They have a team assigned AND + # 2. That team is not Free Agency (abbrev != 'FA') AND + # 3. That team is not the user's team + if (player.team.abbrev != 'FA' and + player.team.id != builder.team.id): + self.logger.warning(f"Player {player.name} belongs to {player.team.abbrev}, cannot add to {builder.team.abbrev} transaction") + return False, f"{player.name} belongs to {player.team.abbrev} and cannot be added to your transaction" # Parse destination destination_map = { @@ -174,17 +255,36 @@ class DropAddCommands(commands.Cog): to_roster = destination_map.get(destination_str.lower()) if not to_roster: self.logger.error(f"Invalid destination: {destination_str}") - return False + return False, f"Invalid destination: {destination_str}" - # Determine player's current roster status based on their team and roster type - if player.team_id == builder.team.id: - # Player is on the user's team - need to determine which roster - # This would need to be enhanced to check actual roster data - # For now, we'll assume they're coming from Major League - from_roster = RosterType.MAJOR_LEAGUE + # Determine player's current roster status by checking actual roster data + # Note: Minor League players have different team_id than Major League team + self.logger.debug(f"Player {player.name} team_id: {player.team_id}, Builder team_id: {builder.team.id}") + + await builder.load_roster_data() + if builder._current_roster: + # Check which roster section the player is on (regardless of team_id) + player_on_active = any(p.id == player.id for p in builder._current_roster.active_players) + player_on_minor = any(p.id == player.id for p in builder._current_roster.minor_league_players) + player_on_il = any(p.id == player.id for p in builder._current_roster.il_players) + + if player_on_active: + from_roster = RosterType.MAJOR_LEAGUE + self.logger.debug(f"Player {player.name} found on active roster (Major League)") + elif player_on_minor: + from_roster = RosterType.MINOR_LEAGUE + self.logger.debug(f"Player {player.name} found on minor league roster") + elif player_on_il: + from_roster = RosterType.INJURED_LIST + self.logger.debug(f"Player {player.name} found on injured list") + else: + # Player not found on user's roster - they're from another team or free agency + from_roster = RosterType.FREE_AGENCY + self.logger.debug(f"Player {player.name} not found on user's roster, treating as free agency") else: - # Player is on another team or free agency + # Couldn't load roster data, assume free agency as safest fallback from_roster = RosterType.FREE_AGENCY + self.logger.warning(f"Could not load roster data, assuming {player.name} is free agency") # Create move move = TransactionMove( @@ -198,11 +298,12 @@ class DropAddCommands(commands.Cog): success, error_message = builder.add_move(move) if not success: self.logger.warning(f"Failed to add quick move: {error_message}") - return success - + return False, error_message + return True, "" + except Exception as e: self.logger.error(f"Error adding quick move: {e}") - return False + return False, f"Error adding move: {str(e)}" @app_commands.command( name="cleartransaction", @@ -218,56 +319,6 @@ class DropAddCommands(commands.Cog): ephemeral=True ) - @app_commands.command( - name="transactionstatus", - description="Show your current transaction builder status" - ) - @logged_command("/transactionstatus") - async def transaction_status(self, interaction: discord.Interaction): - """Show the current status of user's transaction builder.""" - await interaction.response.defer(ephemeral=True) - - # Get user's major league team - major_league_teams = await team_service.get_teams_by_owner( - interaction.user.id, - SBA_CURRENT_SEASON, - roster_type="ml" - ) - - if not major_league_teams: - await interaction.followup.send( - "āŒ You don't appear to own a major league team in the current season.", - ephemeral=True - ) - return - - team = major_league_teams[0] - builder = get_transaction_builder(interaction.user.id, team) - - if builder.is_empty: - await interaction.followup.send( - "šŸ“‹ Your transaction builder is empty. Use `/dropadd` to start building!", - ephemeral=True - ) - return - - # Show current status - validation = await builder.validate_transaction() - - status_msg = f"šŸ“‹ **Transaction Builder Status - {team.abbrev}**\n\n" - status_msg += f"**Moves:** {builder.move_count}\n" - status_msg += f"**Status:** {'āœ… Legal' if validation.is_legal else 'āŒ Illegal'}\n\n" - - if validation.errors: - status_msg += "**Errors:**\n" - status_msg += "\n".join([f"ā€¢ {error}" for error in validation.errors]) - status_msg += "\n\n" - - if validation.suggestions: - status_msg += "**Suggestions:**\n" - status_msg += "\n".join([f"šŸ’” {suggestion}" for suggestion in validation.suggestions]) - - await interaction.followup.send(status_msg, ephemeral=True) async def setup(bot): diff --git a/tests/test_commands_dropadd.py b/tests/test_commands_dropadd.py index 9ee7d85..80ef9f3 100644 --- a/tests/test_commands_dropadd.py +++ b/tests/test_commands_dropadd.py @@ -13,6 +13,7 @@ from services.transaction_builder import TransactionBuilder from models.team import RosterType from models.team import Team from models.player import Player +from tests.factories import PlayerFactory, TeamFactory class TestDropAddCommands: @@ -47,34 +48,23 @@ class TestDropAddCommands: @pytest.fixture def mock_team(self): """Create mock team data.""" - return Team( - id=499, - abbrev='WV', - sname='Black Bears', - lname='West Virginia Black Bears', - season=12 - ) + return TeamFactory.west_virginia() @pytest.fixture def mock_player(self): """Create mock player data.""" - return Player( - id=12472, - name='Mike Trout', - season=12, - primary_position='CF' - ) + return PlayerFactory.mike_trout() @pytest.mark.asyncio async def test_player_autocomplete_success(self, commands_cog, mock_interaction): """Test successful player autocomplete.""" mock_players = [ - Player(id=1, name='Mike Trout', season=12, primary_position='CF'), - Player(id=2, name='Ronald Acuna Jr.', season=12, primary_position='OF') + PlayerFactory.mike_trout(id=1), + PlayerFactory.ronald_acuna(id=2) ] with patch('commands.transactions.dropadd.player_service') as mock_service: - mock_service.get_players_by_name.return_value = mock_players + mock_service.search_players = AsyncMock(return_value=mock_players) choices = await commands_cog.player_autocomplete(mock_interaction, 'Trout') @@ -87,18 +77,12 @@ class TestDropAddCommands: @pytest.mark.asyncio async def test_player_autocomplete_with_team(self, commands_cog, mock_interaction): """Test player autocomplete with team information.""" - mock_team = Team(id=499, abbrev='LAA', sname='Angels', lname='Los Angeles Angels', season=12) - mock_player = Player( - id=1, - name='Mike Trout', - season=12, - primary_position='CF' - ) + mock_team = TeamFactory.create(id=499, abbrev='LAA', sname='Angels', lname='Los Angeles Angels') + mock_player = PlayerFactory.mike_trout(id=1) mock_player.team = mock_team # Add team info with patch('commands.transactions.dropadd.player_service') as mock_service: - mock_service.get_players_by_name.return_value = [mock_player] - + mock_service.search_players = AsyncMock(return_value=[mock_player]) choices = await commands_cog.player_autocomplete(mock_interaction, 'Trout') assert len(choices) == 1 @@ -115,7 +99,7 @@ class TestDropAddCommands: async def test_player_autocomplete_error_handling(self, commands_cog, mock_interaction): """Test player autocomplete error handling.""" with patch('commands.transactions.dropadd.player_service') as mock_service: - mock_service.get_players_by_name.side_effect = Exception("API Error") + mock_service.search_players.side_effect = Exception("API Error") choices = await commands_cog.player_autocomplete(mock_interaction, 'Trout') assert len(choices) == 0 @@ -124,16 +108,15 @@ class TestDropAddCommands: async def test_dropadd_command_no_team(self, commands_cog, mock_interaction): """Test /dropadd command when user has no team.""" with patch('commands.transactions.dropadd.team_service') as mock_service: - mock_service.get_teams_by_owner.return_value = [] - - await commands_cog.dropadd(mock_interaction) + mock_service.get_teams_by_owner = AsyncMock(return_value=[]) + await commands_cog.dropadd.callback(commands_cog, mock_interaction) mock_interaction.response.defer.assert_called_once() mock_interaction.followup.send.assert_called_once() # Check error message call_args = mock_interaction.followup.send.call_args - assert "don't appear to own a team" in call_args[0][0] + assert "don't appear to own" in call_args[0][0] assert call_args[1]['ephemeral'] is True @pytest.mark.asyncio @@ -142,7 +125,7 @@ class TestDropAddCommands: with patch('commands.transactions.dropadd.team_service') as mock_team_service: with patch('commands.transactions.dropadd.get_transaction_builder') as mock_get_builder: with patch('commands.transactions.dropadd.create_transaction_embed') as mock_create_embed: - mock_team_service.get_teams_by_owner.return_value = [mock_team] + mock_team_service.get_teams_by_owner = AsyncMock(return_value=[mock_team]) mock_builder = MagicMock() mock_builder.team = mock_team @@ -151,12 +134,12 @@ class TestDropAddCommands: mock_embed = MagicMock() mock_create_embed.return_value = mock_embed - await commands_cog.dropadd(mock_interaction) + await commands_cog.dropadd.callback(commands_cog, mock_interaction) # Verify flow mock_interaction.response.defer.assert_called_once() mock_team_service.get_teams_by_owner.assert_called_once_with( - mock_interaction.user.id, 12 + mock_interaction.user.id, 12, roster_type='ml' ) mock_get_builder.assert_called_once_with(mock_interaction.user.id, mock_team) mock_create_embed.assert_called_once_with(mock_builder) @@ -169,23 +152,23 @@ class TestDropAddCommands: with patch('commands.transactions.dropadd.get_transaction_builder') as mock_get_builder: with patch.object(commands_cog, '_add_quick_move') as mock_add_quick: with patch('commands.transactions.dropadd.create_transaction_embed') as mock_create_embed: - mock_team_service.get_teams_by_owner.return_value = [mock_team] + mock_team_service.get_teams_by_owner = AsyncMock(return_value=[mock_team]) mock_builder = MagicMock() + mock_builder.move_count = 1 mock_get_builder.return_value = mock_builder - mock_add_quick.return_value = True + mock_add_quick.return_value = (True, "") mock_create_embed.return_value = MagicMock() - await commands_cog.dropadd( - mock_interaction, + await commands_cog.dropadd.callback(commands_cog, + mock_interaction, player='Mike Trout', - action='add', destination='ml' ) # Verify quick move was attempted mock_add_quick.assert_called_once_with( - mock_builder, 'Mike Trout', 'add', 'ml' + mock_builder, 'Mike Trout', 'ml' ) @pytest.mark.asyncio @@ -193,17 +176,23 @@ 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.return_value = (True, "") + mock_builder.load_roster_data = AsyncMock() + mock_builder._current_roster = MagicMock() + mock_builder._current_roster.active_players = [] + mock_builder._current_roster.minor_league_players = [] + mock_builder._current_roster.il_players = [] + with patch('commands.transactions.dropadd.player_service') as mock_service: - mock_service.get_players_by_name.return_value = [mock_player] - - success = await commands_cog._add_quick_move( - mock_builder, 'Mike Trout', 'add', 'ml' + mock_service.search_players = AsyncMock(return_value=[mock_player]) + + success, error_message = await commands_cog._add_quick_move( + mock_builder, 'Mike Trout', 'ml' ) - + assert success is True - mock_service.get_players_by_name.assert_called_once_with('Mike Trout', 12) + assert error_message == "" + mock_service.search_players.assert_called_once_with('Mike Trout', limit=10, season=12) mock_builder.add_move.assert_called_once() @pytest.mark.asyncio @@ -213,26 +202,86 @@ class TestDropAddCommands: mock_builder.team = mock_team with patch('commands.transactions.dropadd.player_service') as mock_service: - mock_service.get_players_by_name.return_value = [] + mock_service.search_players = AsyncMock(return_value=[]) - success = await commands_cog._add_quick_move( - mock_builder, 'Nonexistent Player', 'add', 'ml' + success, error_message = await commands_cog._add_quick_move( + mock_builder, 'Nonexistent Player', 'ml' ) - + assert success is False + assert "not found" in error_message @pytest.mark.asyncio - async def test_add_quick_move_invalid_action(self, commands_cog, mock_team): + async def test_add_quick_move_invalid_action(self, commands_cog, mock_team, mock_player): """Test quick move with invalid action.""" mock_builder = MagicMock() mock_builder.team = mock_team - - success = await commands_cog._add_quick_move( - mock_builder, 'Mike Trout', 'invalid_action', 'ml' - ) - - assert success is False - + + with patch('commands.transactions.dropadd.player_service') as mock_service: + mock_service.search_players = AsyncMock(return_value=[mock_player]) + + success, error_message = await commands_cog._add_quick_move( + mock_builder, 'Mike Trout', 'invalid_destination' + ) + + assert success is False + assert "Invalid destination" in error_message + + @pytest.mark.asyncio + async def test_add_quick_move_player_from_other_team(self, commands_cog, mock_team): + """Test quick move when player belongs to another team.""" + mock_builder = MagicMock() + mock_builder.team = mock_team # West Virginia (team_id=499) + + # Create a player from another team (not Free Agency) + other_team = MagicMock() + other_team.id = 100 # Different team + other_team.abbrev = "LAA" # Not 'FA' + + mock_player = MagicMock() + mock_player.name = "Mike Trout" + mock_player.team = other_team + + with patch('commands.transactions.dropadd.player_service') as mock_service: + mock_service.search_players = AsyncMock(return_value=[mock_player]) + + success, error_message = await commands_cog._add_quick_move( + mock_builder, 'Mike Trout', 'ml' + ) + + assert success is False + assert "belongs to LAA" in error_message + assert "cannot be added to your transaction" in error_message + + @pytest.mark.asyncio + async def test_add_quick_move_free_agent_allowed(self, commands_cog, mock_team): + """Test quick move when player is a Free Agent (should be allowed).""" + from tests.factories import PlayerFactory, TeamFactory + + mock_builder = MagicMock() + mock_builder.team = mock_team + mock_builder.add_move.return_value = (True, "") + mock_builder.load_roster_data = AsyncMock() + mock_builder._current_roster = MagicMock() + mock_builder._current_roster.active_players = [] + mock_builder._current_roster.minor_league_players = [] + mock_builder._current_roster.il_players = [] + + # Create a Free Agent team and player + fa_team = TeamFactory.create(id=1, abbrev="FA", sname="Free Agency", lname="Free Agency") + fa_player = PlayerFactory.create(id=12472, name="Mike Trout", team_id=1) + fa_player.team = fa_team + + with patch('commands.transactions.dropadd.player_service') as mock_service: + mock_service.search_players = AsyncMock(return_value=[fa_player]) + + success, error_message = await commands_cog._add_quick_move( + mock_builder, 'Mike Trout', 'ml' + ) + + assert success is True + assert error_message == "" + # TODO: These tests are for obsolete MoveAction-based functionality # The transaction system now uses from_roster/to_roster directly # def test_determine_roster_types_add(self, commands_cog): @@ -245,7 +294,7 @@ class TestDropAddCommands: async def test_clear_transaction_command(self, commands_cog, mock_interaction): """Test /cleartransaction command.""" with patch('commands.transactions.dropadd.clear_transaction_builder') as mock_clear: - await commands_cog.clear_transaction(mock_interaction) + await commands_cog.clear_transaction.callback(commands_cog, mock_interaction) mock_clear.assert_called_once_with(mock_interaction.user.id) mock_interaction.response.send_message.assert_called_once() @@ -255,64 +304,79 @@ class TestDropAddCommands: assert "transaction builder has been cleared" in call_args[0][0] assert call_args[1]['ephemeral'] is True + @pytest.mark.asyncio - async def test_transaction_status_no_team(self, commands_cog, mock_interaction): - """Test /transactionstatus when user has no team.""" - with patch('commands.transactions.dropadd.team_service') as mock_service: - mock_service.get_teams_by_owner.return_value = [] - - await commands_cog.transaction_status(mock_interaction) - - mock_interaction.response.defer.assert_called_once_with(ephemeral=True) - mock_interaction.followup.send.assert_called_once() - - call_args = mock_interaction.followup.send.call_args - assert "don't appear to own a team" in call_args[0][0] - - @pytest.mark.asyncio - async def test_transaction_status_empty_builder(self, commands_cog, mock_interaction, mock_team): - """Test /transactionstatus with empty builder.""" + async def test_dropadd_first_move_shows_full_embed(self, commands_cog, mock_interaction, mock_team): + """Test /dropadd command with first move shows full interactive embed.""" with patch('commands.transactions.dropadd.team_service') as mock_team_service: with patch('commands.transactions.dropadd.get_transaction_builder') as mock_get_builder: - mock_team_service.get_teams_by_owner.return_value = [mock_team] - - mock_builder = MagicMock() - mock_builder.is_empty = True - mock_get_builder.return_value = mock_builder - - await commands_cog.transaction_status(mock_interaction) - - call_args = mock_interaction.followup.send.call_args - assert "transaction builder is empty" in call_args[0][0] - + with patch.object(commands_cog, '_add_quick_move') as mock_add_quick: + with patch('commands.transactions.dropadd.create_transaction_embed') as mock_create_embed: + mock_team_service.get_teams_by_owner = AsyncMock(return_value=[mock_team]) + + # Create empty builder (first move) + mock_builder = MagicMock() + mock_builder.is_empty = True + mock_builder.move_count = 1 + mock_get_builder.return_value = mock_builder + mock_add_quick.return_value = (True, "") + mock_create_embed.return_value = MagicMock() + + await commands_cog.dropadd.callback(commands_cog, + mock_interaction, + player='Mike Trout', + destination='ml' + ) + + # Should show full embed with view (now ephemeral) + mock_interaction.followup.send.assert_called_once() + call_args = mock_interaction.followup.send.call_args + assert call_args[1]['ephemeral'] is True + assert 'embed' in call_args[1] + assert 'view' in call_args[1] + assert 'content' in call_args[1] + @pytest.mark.asyncio - async def test_transaction_status_with_moves(self, commands_cog, mock_interaction, mock_team): - """Test /transactionstatus with moves in builder.""" - from services.transaction_builder import RosterValidationResult - + async def test_dropadd_append_mode_shows_confirmation(self, commands_cog, mock_interaction, mock_team): + """Test /dropadd command in append mode shows ephemeral confirmation.""" with patch('commands.transactions.dropadd.team_service') as mock_team_service: with patch('commands.transactions.dropadd.get_transaction_builder') as mock_get_builder: - mock_team_service.get_teams_by_owner.return_value = [mock_team] - - mock_builder = MagicMock() - mock_builder.is_empty = False - mock_builder.move_count = 2 - mock_builder.validate_transaction = AsyncMock(return_value=RosterValidationResult( - is_legal=True, - major_league_count=25, - minor_league_count=10, - warnings=[], - errors=[], - suggestions=[] - )) - mock_get_builder.return_value = mock_builder - - await commands_cog.transaction_status(mock_interaction) - - call_args = mock_interaction.followup.send.call_args - status_msg = call_args[0][0] - assert "Moves:** 2" in status_msg - assert "āœ… Legal" in status_msg + with patch.object(commands_cog, '_add_quick_move') as mock_add_quick: + mock_team_service.get_teams_by_owner = AsyncMock(return_value=[mock_team]) + + # Create builder with existing moves (append mode) + mock_builder = MagicMock() + mock_builder.is_empty = False + mock_builder.move_count = 2 + mock_builder.validate_transaction = AsyncMock(return_value=MagicMock( + is_legal=True, + major_league_count=25, + minor_league_count=10, + warnings=[], + errors=[], + suggestions=[] + )) + mock_get_builder.return_value = mock_builder + mock_add_quick.return_value = (True, "") + + with patch('commands.transactions.dropadd.create_transaction_embed') as mock_create_embed: + mock_create_embed.return_value = MagicMock() + + await commands_cog.dropadd.callback(commands_cog, + mock_interaction, + player='Kevin Ginkel', + destination='ml' + ) + + # Should show embed with ephemeral confirmation + mock_interaction.followup.send.assert_called_once() + call_args = mock_interaction.followup.send.call_args + assert call_args[1]['ephemeral'] is True + assert 'embed' in call_args[1] + assert 'view' in call_args[1] + content = call_args[1]['content'] + assert "Added Kevin Ginkel ā†’ ML" in content + assert "Transaction now has 2 moves" in content class TestDropAddCommandsIntegration: @@ -334,38 +398,30 @@ class TestDropAddCommandsIntegration: mock_interaction = AsyncMock() mock_interaction.user.id = 123456789 - mock_team = Team( - id=499, - abbrev='WV', - sname='Black Bears', - lname='West Virginia Black Bears', - season=12 - ) + mock_team = TeamFactory.west_virginia() - mock_player = Player( - id=12472, - name='Mike Trout', - season=12, - primary_position='CF' - ) + mock_player = PlayerFactory.mike_trout(id=12472) with patch('commands.transactions.dropadd.team_service') as mock_team_service: with patch('commands.transactions.dropadd.player_service') as mock_player_service: with patch('commands.transactions.dropadd.get_transaction_builder') as mock_get_builder: with patch('commands.transactions.dropadd.create_transaction_embed') as mock_create_embed: # Setup mocks - mock_team_service.get_teams_by_owner.return_value = [mock_team] - mock_player_service.get_players_by_name.return_value = [mock_player] + mock_team_service.get_teams_by_owner = AsyncMock(return_value=[mock_team]) + mock_player_service.search_players = AsyncMock(return_value=[mock_player]) mock_builder = TransactionBuilder(mock_team, 123456789, 12) mock_get_builder.return_value = mock_builder - mock_create_embed.return_value = MagicMock() + + # Mock the async function + async def mock_create_embed_func(builder): + return MagicMock() + mock_create_embed.side_effect = mock_create_embed_func # Execute command with parameters - await commands_cog.dropadd( + await commands_cog.dropadd.callback(commands_cog, mock_interaction, player='Mike Trout', - action='add', destination='ml' ) @@ -384,10 +440,11 @@ class TestDropAddCommandsIntegration: with patch('commands.transactions.dropadd.team_service') as mock_service: # Simulate API error - mock_service.get_teams_by_owner.side_effect = Exception("API Error") + mock_service.get_teams_by_owner = AsyncMock(side_effect=Exception("API Error")) - # Should not raise exception, should handle gracefully - await commands_cog.dropadd(mock_interaction) - - # Should have deferred and attempted to send error (which will also fail gracefully) + # Exception should be raised (logged_command decorator re-raises) + with pytest.raises(Exception, match="API Error"): + await commands_cog.dropadd.callback(commands_cog, mock_interaction) + + # Should have deferred before the error occurred mock_interaction.response.defer.assert_called_once() \ No newline at end of file