From bf079eab83b89128bf0b722b6d06fe1dc2cf3552 Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Fri, 23 Jan 2026 14:23:16 -0600 Subject: [PATCH] Add all-season player search with deduplicated autocomplete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - PlayerService.search_players() now supports all_seasons=True to search across all 13 seasons - Autocomplete shows unique player names (most recent season's team) instead of duplicates - Command defaults to most recent season when no season parameter specified - Users can specify season parameter for historical data 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- commands/players/info.py | 218 +++++++++++----- services/player_service.py | 199 ++++++++++----- tests/test_services_player_service.py | 351 +++++++++++++++----------- 3 files changed, 481 insertions(+), 287 deletions(-) diff --git a/commands/players/info.py b/commands/players/info.py index b282cf3..2ebeba6 100644 --- a/commands/players/info.py +++ b/commands/players/info.py @@ -3,6 +3,7 @@ Player Information Commands Implements slash commands for displaying player information and statistics. """ + from typing import Optional, List import discord @@ -21,26 +22,50 @@ async def player_name_autocomplete( interaction: discord.Interaction, current: str, ) -> List[discord.app_commands.Choice[str]]: - """Autocomplete for player names.""" + """Autocomplete for player names across all seasons, deduplicated by name. + + Returns unique player names only (most recent season's data for display). + Users can specify the `season` parameter to get historical data. + """ if len(current) < 2: return [] try: - # Use the dedicated search endpoint to get matching players - players = await player_service.search_players(current, limit=25, season=get_config().sba_season) + # Use the dedicated search endpoint to get matching players across ALL seasons + # Results are ordered by most recent season first + players = await player_service.search_players( + current, + limit=50, + all_seasons=True, # Fetch more to ensure 25 unique names + ) - # Convert to discord choices, limiting to 25 (Discord's max) + # Deduplicate by player name, keeping only the first (most recent) occurrence + seen_names: set[str] = set() choices = [] - for player in players[:25]: + + for player in players: + # Skip if we've already seen this player name + name_lower = player.name.lower() + if name_lower in seen_names: + continue + seen_names.add(name_lower) + # Format: "Player Name (Position) - Team" + # No season indicator needed since we're showing unique players display_name = f"{player.name} ({player.primary_position})" - if hasattr(player, 'team') and player.team: + if hasattr(player, "team") and player.team: display_name += f" - {player.team.abbrev}" - choices.append(discord.app_commands.Choice( - name=display_name, - value=player.name - )) + choices.append( + discord.app_commands.Choice( + name=display_name[:100], # Discord limit is 100 chars + value=player.name, + ) + ) + + # Stop once we have 25 unique players (Discord's max) + if len(choices) >= 25: + break return choices @@ -51,109 +76,170 @@ async def player_name_autocomplete( class PlayerInfoCommands(commands.Cog): """Player information and statistics command handlers.""" - + def __init__(self, bot: commands.Bot): self.bot = bot - self.logger = get_contextual_logger(f'{__name__}.PlayerInfoCommands') - + self.logger = get_contextual_logger(f"{__name__}.PlayerInfoCommands") + @discord.app_commands.command( name="player", - description="Display player information and statistics" + description="Display player information and statistics", ) @discord.app_commands.describe( name="Player name to search for", - season="Season to show stats for (defaults to current season)" + season="Season number for historical stats (defaults to most recent)", ) @discord.app_commands.autocomplete(name=player_name_autocomplete) @logged_command("/player") async def player_info( - self, - interaction: discord.Interaction, - name: str, - season: Optional[int] = None + self, interaction: discord.Interaction, name: str, season: Optional[int] = None ): """Display player card with statistics.""" # Defer response for potentially slow API calls await interaction.response.defer() self.logger.debug("Response deferred") - - # Search for player by name (use season parameter or default to current) - search_season = season or get_config().sba_season - self.logger.debug("Starting player search", api_call="get_players_by_name", season=search_season) - players = await player_service.get_players_by_name(name, search_season) - self.logger.info("Player search completed", players_found=len(players), season=search_season) - + + # Determine search strategy based on whether season was explicitly provided + explicit_season = season is not None + + if explicit_season: + # User specified a season - search only that season + search_season = season + self.logger.debug( + "Starting player search (explicit season)", + api_call="get_players_by_name", + season=search_season, + ) + players = await player_service.get_players_by_name(name, search_season) + else: + # No season specified - search ALL seasons, prioritize most recent + self.logger.debug( + "Starting player search (all seasons)", + api_call="search_players", + all_seasons=True, + ) + players = await player_service.search_players( + name, limit=25, all_seasons=True + ) + # Set search_season to the season of the first matching player (most recent) + search_season = players[0].season if players else get_config().sba_season + + self.logger.info( + "Player search completed", + players_found=len(players), + season=search_season, + all_seasons=not explicit_season, + ) + if not players: - # Try fuzzy search as fallback - self.logger.info("No exact matches found, attempting fuzzy search", search_term=name) - fuzzy_players = await player_service.search_players_fuzzy(name, limit=10) - + # Try fuzzy search as fallback (search all seasons) + self.logger.info( + "No exact matches found, attempting fuzzy search", search_term=name + ) + fuzzy_players = await player_service.search_players_fuzzy( + name, limit=10, all_seasons=True + ) + if not fuzzy_players: - self.logger.warning("No players found even with fuzzy search", search_term=name) + self.logger.warning( + "No players found even with fuzzy search", search_term=name + ) await interaction.followup.send( - f"❌ No players found matching '{name}'.", - ephemeral=True + f"❌ No players found matching '{name}'.", ephemeral=True ) return - - # Show fuzzy search results for user selection - self.logger.info("Fuzzy search results found", fuzzy_results_count=len(fuzzy_players)) - fuzzy_list = "\n".join([f"• {p.name} ({p.primary_position})" for p in fuzzy_players[:10]]) + + # Show fuzzy search results for user selection with season info + self.logger.info( + "Fuzzy search results found", fuzzy_results_count=len(fuzzy_players) + ) + fuzzy_list = "\n".join( + [ + f"• {p.name} ({p.primary_position}) [Season {p.season}]" + for p in fuzzy_players[:10] + ] + ) await interaction.followup.send( f"🔍 No exact match found for '{name}'. Did you mean one of these?\n{fuzzy_list}\n\nPlease try again with the exact name.", - ephemeral=True + ephemeral=True, ) return - + # If multiple players, try exact match first player = None if len(players) == 1: player = players[0] - self.logger.debug("Single player found", player_id=player.id, player_name=player.name) + self.logger.debug( + "Single player found", player_id=player.id, player_name=player.name + ) else: - self.logger.debug("Multiple players found, attempting exact match", candidate_count=len(players)) - + self.logger.debug( + "Multiple players found, attempting exact match", + candidate_count=len(players), + ) + # Try exact match for p in players: if p.name.lower() == name.lower(): player = p - self.logger.debug("Exact match found", player_id=player.id, player_name=player.name) + self.logger.debug( + "Exact match found", + player_id=player.id, + player_name=player.name, + ) break - + if player is None: - # Show multiple options - candidate_names = [p.name for p in players[:10]] - self.logger.info("Multiple candidates found, requiring user clarification", - candidates=candidate_names) - - player_list = "\n".join([f"• {p.name} ({p.primary_position})" for p in players[:10]]) + # Show multiple options with season info + candidate_names = [f"{p.name} (S{p.season})" for p in players[:10]] + self.logger.info( + "Multiple candidates found, requiring user clarification", + candidates=candidate_names, + ) + + player_list = "\n".join( + [ + f"• {p.name} ({p.primary_position}) [Season {p.season}]" + for p in players[:10] + ] + ) await interaction.followup.send( - f"🔍 Multiple players found for '{name}':\n{player_list}\n\nPlease be more specific.", - ephemeral=True + f"🔍 Multiple players found for '{name}':\n{player_list}\n\nPlease specify a season with the `season` parameter.", + ephemeral=True, ) return - + # Get player data and statistics concurrently - self.logger.debug("Fetching player data and statistics", - player_id=player.id, - season=search_season) - + self.logger.debug( + "Fetching player data and statistics", + player_id=player.id, + season=search_season, + ) + # Fetch player data and stats concurrently for better performance import asyncio + player_with_team, (batting_stats, pitching_stats) = await asyncio.gather( player_service.get_player(player.id), - stats_service.get_player_stats(player.id, search_season) + stats_service.get_player_stats(player.id, search_season), ) - + if player_with_team is None: self.logger.warning("Failed to get player data, using search result") player_with_team = player # Fallback to search result else: - team_info = f"{player_with_team.team.abbrev}" if hasattr(player_with_team, 'team') and player_with_team.team else "No team" - self.logger.debug("Player data retrieved", team=team_info, - batting_stats=bool(batting_stats), - pitching_stats=bool(pitching_stats)) - + team_info = ( + f"{player_with_team.team.abbrev}" + if hasattr(player_with_team, "team") and player_with_team.team + else "No team" + ) + self.logger.debug( + "Player data retrieved", + team=team_info, + batting_stats=bool(batting_stats), + pitching_stats=bool(pitching_stats), + ) + # Create interactive player view with toggleable statistics self.logger.debug("Creating PlayerStatsView with toggleable statistics") view = PlayerStatsView( @@ -161,7 +247,7 @@ class PlayerInfoCommands(commands.Cog): season=search_season, batting_stats=batting_stats, pitching_stats=pitching_stats, - user_id=None # setting to None so any GM can toggle the stats views + user_id=None, # setting to None so any GM can toggle the stats views ) # Get initial embed with stats hidden @@ -173,4 +259,4 @@ class PlayerInfoCommands(commands.Cog): async def setup(bot: commands.Bot): """Load the player info commands cog.""" - await bot.add_cog(PlayerInfoCommands(bot)) \ No newline at end of file + await bot.add_cog(PlayerInfoCommands(bot)) diff --git a/services/player_service.py b/services/player_service.py index 117cd94..0927b16 100644 --- a/services/player_service.py +++ b/services/player_service.py @@ -3,6 +3,7 @@ Player service for Discord Bot v2.0 Handles player-related operations with team population and search functionality. """ + import logging from typing import Optional, List, TYPE_CHECKING @@ -14,13 +15,13 @@ from exceptions import APIException if TYPE_CHECKING: from services.team_service import TeamService -logger = logging.getLogger(f'{__name__}.PlayerService') +logger = logging.getLogger(f"{__name__}.PlayerService") class PlayerService(BaseService[Player]): """ Service for player-related operations. - + Features: - Player retrieval with team population - Team roster queries @@ -28,20 +29,20 @@ class PlayerService(BaseService[Player]): - Season-specific filtering - Free agent handling via constants """ - - def __init__(self, team_service: Optional['TeamService'] = None): + + def __init__(self, team_service: Optional["TeamService"] = None): """Initialize player service.""" - super().__init__(Player, 'players') + super().__init__(Player, "players") self._team_service = team_service logger.debug("PlayerService initialized") - + async def get_player(self, player_id: int) -> Optional[Player]: """ Get player by ID with error handling. - + Args: player_id: Unique player identifier - + Returns: Player instance or None if not found """ @@ -53,9 +54,10 @@ class PlayerService(BaseService[Player]): except Exception as e: logger.error(f"Unexpected error getting player {player_id}: {e}") return None - - - async def get_players_by_team(self, team_id: int, season: int, sort: Optional[str] = None) -> List[Player]: + + async def get_players_by_team( + self, team_id: int, season: int, sort: Optional[str] = None + ) -> List[Player]: """ Get all players for a specific team. @@ -68,100 +70,112 @@ class PlayerService(BaseService[Player]): List of players on the team, optionally sorted """ try: - params = [ - ('season', str(season)), - ('team_id', str(team_id)) - ] + params = [("season", str(season)), ("team_id", str(team_id))] # Add sort parameter if specified if sort: - valid_sorts = ['cost-asc', 'cost-desc', 'name-asc', 'name-desc'] + valid_sorts = ["cost-asc", "cost-desc", "name-asc", "name-desc"] if sort in valid_sorts: - params.append(('sort', sort)) + params.append(("sort", sort)) logger.debug(f"Applying sort '{sort}' to team {team_id} players") else: logger.warning(f"Invalid sort parameter '{sort}' - ignoring") players = await self.get_all_items(params=params) - logger.debug(f"Retrieved {len(players)} players for team {team_id} in season {season}") + logger.debug( + f"Retrieved {len(players)} players for team {team_id} in season {season}" + ) return players except Exception as e: logger.error(f"Failed to get players for team {team_id}: {e}") return [] - + async def get_players_by_name(self, name: str, season: int) -> List[Player]: """ Search for players by name (partial match). - + Args: name: Player name or partial name season: Season number (required) - + Returns: List of matching players """ try: - params = [ - ('season', str(season)), - ('name', name) - ] - + params = [("season", str(season)), ("name", name)] + players = await self.get_all_items(params=params) - logger.debug(f"Found {len(players)} players matching '{name}' in season {season}") + logger.debug( + f"Found {len(players)} players matching '{name}' in season {season}" + ) return players - + except Exception as e: logger.error(f"Failed to search players by name '{name}': {e}") return [] - - async def get_player_by_name_exact(self, name: str, season: int) -> Optional[Player]: + + async def get_player_by_name_exact( + self, name: str, season: int + ) -> Optional[Player]: """ Get player by exact name match (case-insensitive). - + Args: name: Exact player name season: Season number (required) - + Returns: Player instance or None if not found """ try: players = await self.get_players_by_name(name, season) - + # Look for exact case-insensitive match name_lower = name.lower() for player in players: if player.name.lower() == name_lower: logger.debug(f"Found exact match for '{name}': {player.name}") return player - + logger.debug(f"No exact match found for '{name}'") return None - + except Exception as e: logger.error(f"Error finding exact player match for '{name}': {e}") return None - - async def search_players(self, query: str, limit: int = 10, season: Optional[int] = None) -> List[Player]: + + async def search_players( + self, + query: str, + limit: int = 10, + season: Optional[int] = None, + all_seasons: bool = False, + ) -> List[Player]: """ Search for players using the dedicated /v3/players/search endpoint. Args: query: Search query for player name limit: Maximum number of results to return (1-50) - season: Season to search in (defaults to current season) + season: Season to search in (defaults to current season if all_seasons=False) + all_seasons: If True, search across all seasons (ignores season parameter) Returns: List of matching players (up to limit) """ try: - params = [('q', query), ('limit', str(limit))] - if season is not None: - params.append(('season', str(season))) + params = [("q", query), ("limit", str(limit))] + + if all_seasons: + # Pass season=0 to API to search all seasons + params.append(("season", "0")) + elif season is not None: + params.append(("season", str(season))) + # If neither all_seasons nor season specified, API defaults to current season client = await self.get_client() - data = await client.get('players/search', params=params) + data = await client.get("players/search", params=params) if not data: logger.debug(f"No players found for search query '{query}'") @@ -171,16 +185,50 @@ class PlayerService(BaseService[Player]): items, count = self._extract_items_and_count_from_response(data) players = [self.model_class.from_api_data(item) for item in items] - logger.debug(f"Search '{query}' returned {len(players)} of {count} matches") + logger.debug( + f"Search '{query}' returned {len(players)} of {count} matches (all_seasons={all_seasons})" + ) return players except Exception as e: logger.error(f"Error in player search for '{query}': {e}") return [] - async def search_players_fuzzy(self, query: str, limit: int = 10, season: Optional[int] = None) -> List[Player]: + async def search_players_fuzzy( + self, + query: str, + limit: int = 10, + season: Optional[int] = None, + all_seasons: bool = False, + ) -> List[Player]: """ - Fuzzy search for players by name with limit using existing name search functionality. + Fuzzy search for players by name with limit using the search endpoint. + + Args: + query: Search query + limit: Maximum results to return + season: Season to search in (defaults to current season if all_seasons=False) + all_seasons: If True, search across all seasons + + Returns: + List of matching players (up to limit) + """ + try: + # Use the search endpoint which handles fuzzy matching and all-seasons + return await self.search_players( + query, limit=limit, season=season, all_seasons=all_seasons + ) + + except Exception as e: + logger.error(f"Error in fuzzy search for '{query}': {e}") + return [] + + async def _search_players_fuzzy_legacy( + self, query: str, limit: int = 10, season: Optional[int] = None + ) -> List[Player]: + """ + Legacy fuzzy search for players by name with limit using existing name search functionality. + Kept for backwards compatibility if needed. Args: query: Search query @@ -213,35 +261,39 @@ class PlayerService(BaseService[Player]): results = exact_matches + partial_matches limited_results = results[:limit] - logger.debug(f"Fuzzy search '{query}' returned {len(limited_results)} of {len(results)} matches") + logger.debug( + f"Fuzzy search '{query}' returned {len(limited_results)} of {len(results)} matches" + ) return limited_results except Exception as e: logger.error(f"Error in fuzzy search for '{query}': {e}") return [] - - + async def get_free_agents(self, season: int) -> List[Player]: """ Get all free agent players. - + Args: season: Season number (required) - + Returns: List of free agent players """ try: - params = [('team_id', get_config().free_agent_team_id), ('season', str(season))] - + params = [ + ("team_id", get_config().free_agent_team_id), + ("season", str(season)), + ] + players = await self.get_all_items(params=params) logger.debug(f"Retrieved {len(players)} free agents") return players - + except Exception as e: logger.error(f"Failed to get free agents: {e}") return [] - + async def is_free_agent(self, player: Player) -> bool: """ Check if a player is a free agent. @@ -268,7 +320,9 @@ class PlayerService(BaseService[Player]): try: free_agents = await self.get_free_agents(season) # Sort by wara descending and take top N - sorted_fa = sorted(free_agents, key=lambda p: p.wara if p.wara else 0.0, reverse=True) + sorted_fa = sorted( + free_agents, key=lambda p: p.wara if p.wara else 0.0, reverse=True + ) return sorted_fa[:limit] except Exception as e: logger.error(f"Failed to get top free agents: {e}") @@ -277,26 +331,25 @@ class PlayerService(BaseService[Player]): async def get_players_by_position(self, position: str, season: int) -> List[Player]: """ Get players by position. - + Args: position: Player position (e.g., 'C', '1B', 'OF') season: Season number (required) - + Returns: List of players at the position """ try: - params = [('position', position), ('season', str(season))] - + params = [("position", position), ("season", str(season))] + players = await self.get_all_items(params=params) logger.debug(f"Retrieved {len(players)} players at position {position}") return players - + except Exception as e: logger.error(f"Failed to get players by position {position}: {e}") return [] - - + async def update_player(self, player_id: int, updates: dict) -> Optional[Player]: """ Update player information. @@ -318,7 +371,9 @@ class PlayerService(BaseService[Player]): logger.error(f"Failed to update player {player_id}: {e}") return None - async def update_player_team(self, player_id: int, new_team_id: int) -> Optional[Player]: + async def update_player_team( + self, player_id: int, new_team_id: int + ) -> Optional[Player]: """ Update a player's team assignment (for real-time IL moves). @@ -337,14 +392,22 @@ class PlayerService(BaseService[Player]): """ try: logger.info(f"Updating player {player_id} team to {new_team_id}") - updated_player = await self.update_player(player_id, {'team_id': new_team_id}) + updated_player = await self.update_player( + player_id, {"team_id": new_team_id} + ) if updated_player: - logger.info(f"Successfully updated player {player_id} to team {new_team_id}") + logger.info( + f"Successfully updated player {player_id} to team {new_team_id}" + ) return updated_player else: - logger.error(f"Failed to update player {player_id} team - no response from API") - raise APIException(f"Failed to update player {player_id} team assignment") + logger.error( + f"Failed to update player {player_id} team - no response from API" + ) + raise APIException( + f"Failed to update player {player_id} team assignment" + ) except Exception as e: logger.error(f"Error updating player {player_id} team: {e}") @@ -352,4 +415,4 @@ class PlayerService(BaseService[Player]): # Global service instance - will be properly initialized in __init__.py -player_service = PlayerService() \ No newline at end of file +player_service = PlayerService() diff --git a/tests/test_services_player_service.py b/tests/test_services_player_service.py index 14bcbec..0eeaa74 100644 --- a/tests/test_services_player_service.py +++ b/tests/test_services_player_service.py @@ -1,6 +1,7 @@ """ Tests for PlayerService functionality """ + import pytest from unittest.mock import AsyncMock @@ -12,184 +13,211 @@ from exceptions import APIException class TestPlayerService: """Test PlayerService functionality.""" - + @pytest.fixture def mock_client(self): """Mock API client.""" client = AsyncMock() return client - + @pytest.fixture def player_service_instance(self, mock_client): """Create PlayerService instance with mocked client.""" service = PlayerService() service._client = mock_client return service - - def create_player_data(self, player_id: int, name: str, team_id: int = 5, position: str = 'C', **kwargs): + + def create_player_data( + self, player_id: int, name: str, team_id: int = 5, position: str = "C", **kwargs + ): """Create complete player data for testing.""" base_data = { - 'id': player_id, - 'name': name, - 'wara': 2.5, - 'season': 12, - 'team_id': team_id, - 'image': f'https://example.com/player{player_id}.jpg', - 'pos_1': position, + "id": player_id, + "name": name, + "wara": 2.5, + "season": 12, + "team_id": team_id, + "image": f"https://example.com/player{player_id}.jpg", + "pos_1": position, } base_data.update(kwargs) return base_data - + @pytest.mark.asyncio async def test_get_player_success(self, player_service_instance, mock_client): """Test successful player retrieval.""" - mock_data = self.create_player_data(1, 'Test Player', pos_2='1B') + mock_data = self.create_player_data(1, "Test Player", pos_2="1B") mock_client.get.return_value = mock_data - + result = await player_service_instance.get_player(1) - + assert isinstance(result, Player) - assert result.name == 'Test Player' + assert result.name == "Test Player" assert result.wara == 2.5 assert result.season == 12 - assert result.primary_position == 'C' - mock_client.get.assert_called_once_with('players', object_id=1) - + assert result.primary_position == "C" + mock_client.get.assert_called_once_with("players", object_id=1) + @pytest.mark.asyncio - async def test_get_player_includes_team_data(self, player_service_instance, mock_client): + async def test_get_player_includes_team_data( + self, player_service_instance, mock_client + ): """Test that get_player returns data with team information (from API).""" # API returns player data with team information already included - player_data = self.create_player_data(1, 'Test Player', team_id=5) - player_data['team'] = { - 'id': 5, - 'abbrev': 'TST', - 'sname': 'Test Team', - 'lname': 'Test Team Long Name', - 'season': 12 + player_data = self.create_player_data(1, "Test Player", team_id=5) + player_data["team"] = { + "id": 5, + "abbrev": "TST", + "sname": "Test Team", + "lname": "Test Team Long Name", + "season": 12, } - + mock_client.get.return_value = player_data - + result = await player_service_instance.get_player(1) - + assert isinstance(result, Player) - assert result.name == 'Test Player' + assert result.name == "Test Player" assert result.team is not None - assert result.team.sname == 'Test Team' - + assert result.team.sname == "Test Team" + # Should call get once for player (team data included in API response) - mock_client.get.assert_called_once_with('players', object_id=1) - + mock_client.get.assert_called_once_with("players", object_id=1) + @pytest.mark.asyncio async def test_get_players_by_team(self, player_service_instance, mock_client): """Test getting players by team.""" mock_data = { - 'count': 2, - 'players': [ - self.create_player_data(1, 'Player1', team_id=5), - self.create_player_data(2, 'Player2', team_id=5) - ] + "count": 2, + "players": [ + self.create_player_data(1, "Player1", team_id=5), + self.create_player_data(2, "Player2", team_id=5), + ], } mock_client.get.return_value = mock_data - + result = await player_service_instance.get_players_by_team(5, season=12) assert len(result) == 2 assert all(isinstance(p, Player) for p in result) - mock_client.get.assert_called_once_with('players', params=[('season', '12'), ('team_id', '5')]) + mock_client.get.assert_called_once_with( + "players", params=[("season", "12"), ("team_id", "5")] + ) @pytest.mark.asyncio - async def test_get_players_by_team_with_sort(self, player_service_instance, mock_client): + async def test_get_players_by_team_with_sort( + self, player_service_instance, mock_client + ): """Test getting players by team with sort parameter.""" mock_data = { - 'count': 2, - 'players': [ - self.create_player_data(1, 'Player1', team_id=5), - self.create_player_data(2, 'Player2', team_id=5) - ] + "count": 2, + "players": [ + self.create_player_data(1, "Player1", team_id=5), + self.create_player_data(2, "Player2", team_id=5), + ], } mock_client.get.return_value = mock_data # Test with valid sort parameter - result = await player_service_instance.get_players_by_team(5, season=12, sort='cost-asc') + result = await player_service_instance.get_players_by_team( + 5, season=12, sort="cost-asc" + ) assert len(result) == 2 assert all(isinstance(p, Player) for p in result) - mock_client.get.assert_called_once_with('players', params=[('season', '12'), ('team_id', '5'), ('sort', 'cost-asc')]) + mock_client.get.assert_called_once_with( + "players", params=[("season", "12"), ("team_id", "5"), ("sort", "cost-asc")] + ) # Reset mock for next test mock_client.reset_mock() # Test with invalid sort parameter (should be ignored) - result = await player_service_instance.get_players_by_team(5, season=12, sort='invalid-sort') + result = await player_service_instance.get_players_by_team( + 5, season=12, sort="invalid-sort" + ) assert len(result) == 2 # Should not include sort parameter when invalid - mock_client.get.assert_called_once_with('players', params=[('season', '12'), ('team_id', '5')]) + mock_client.get.assert_called_once_with( + "players", params=[("season", "12"), ("team_id", "5")] + ) @pytest.mark.asyncio async def test_get_players_by_name(self, player_service_instance, mock_client): """Test searching players by name.""" mock_data = { - 'count': 1, - 'players': [ - self.create_player_data(1, 'John Smith', team_id=5) - ] + "count": 1, + "players": [self.create_player_data(1, "John Smith", team_id=5)], } mock_client.get.return_value = mock_data - - result = await player_service_instance.get_players_by_name('John', season=13) + + result = await player_service_instance.get_players_by_name("John", season=13) assert len(result) == 1 - assert result[0].name == 'John Smith' - mock_client.get.assert_called_once_with('players', params=[('season', '13'), ('name', 'John')]) - + assert result[0].name == "John Smith" + mock_client.get.assert_called_once_with( + "players", params=[("season", "13"), ("name", "John")] + ) + @pytest.mark.asyncio async def test_get_player_by_name_exact(self, player_service_instance, mock_client): """Test exact name matching.""" mock_data = { - 'count': 2, - 'players': [ - self.create_player_data(1, 'John Smith', team_id=5), - self.create_player_data(2, 'John Doe', team_id=6) - ] + "count": 2, + "players": [ + self.create_player_data(1, "John Smith", team_id=5), + self.create_player_data(2, "John Doe", team_id=6), + ], } mock_client.get.return_value = mock_data - - result = await player_service_instance.get_player_by_name_exact('John Smith', season=12) - + + result = await player_service_instance.get_player_by_name_exact( + "John Smith", season=12 + ) + assert result is not None - assert result.name == 'John Smith' + assert result.name == "John Smith" assert result.id == 1 - + @pytest.mark.asyncio async def test_get_free_agents(self, player_service_instance, mock_client): """Test getting free agents.""" mock_data = { - 'count': 2, - 'players': [ - self.create_player_data(1, 'Free Agent 1', team_id=get_config().free_agent_team_id), - self.create_player_data(2, 'Free Agent 2', team_id=get_config().free_agent_team_id) - ] + "count": 2, + "players": [ + self.create_player_data( + 1, "Free Agent 1", team_id=get_config().free_agent_team_id + ), + self.create_player_data( + 2, "Free Agent 2", team_id=get_config().free_agent_team_id + ), + ], } mock_client.get.return_value = mock_data - + result = await player_service_instance.get_free_agents(season=12) - + assert len(result) == 2 assert all(p.team_id == get_config().free_agent_team_id for p in result) - mock_client.get.assert_called_once_with('players', params=[('team_id', get_config().free_agent_team_id), ('season', '12')]) - + mock_client.get.assert_called_once_with( + "players", + params=[("team_id", get_config().free_agent_team_id), ("season", "12")], + ) + @pytest.mark.asyncio async def test_is_free_agent(self, player_service_instance): """Test free agent checking.""" # Create test players with all required fields - free_agent_data = self.create_player_data(1, 'Free Agent', team_id=get_config().free_agent_team_id) - regular_player_data = self.create_player_data(2, 'Regular Player', team_id=5) - + free_agent_data = self.create_player_data( + 1, "Free Agent", team_id=get_config().free_agent_team_id + ) + regular_player_data = self.create_player_data(2, "Regular Player", team_id=5) + free_agent = Player.from_api_data(free_agent_data) regular_player = Player.from_api_data(regular_player_data) - + assert await player_service_instance.is_free_agent(free_agent) is True assert await player_service_instance.is_free_agent(regular_player) is False @@ -197,153 +225,170 @@ class TestPlayerService: async def test_search_players(self, player_service_instance, mock_client): """Test new search_players functionality using /v3/players/search endpoint.""" mock_players = [ - self.create_player_data(1, 'Mike Trout', pos_1='OF'), - self.create_player_data(2, 'Michael Harris', pos_1='OF') + self.create_player_data(1, "Mike Trout", pos_1="OF"), + self.create_player_data(2, "Michael Harris", pos_1="OF"), ] - mock_client.get.return_value = { - 'count': 2, - 'players': mock_players - } + mock_client.get.return_value = {"count": 2, "players": mock_players} - result = await player_service_instance.search_players('Mike', limit=10, season=12) + result = await player_service_instance.search_players( + "Mike", limit=10, season=12 + ) - mock_client.get.assert_called_once_with('players/search', params=[('q', 'Mike'), ('limit', '10'), ('season', '12')]) + mock_client.get.assert_called_once_with( + "players/search", params=[("q", "Mike"), ("limit", "10"), ("season", "12")] + ) assert len(result) == 2 assert all(isinstance(player, Player) for player in result) - assert result[0].name == 'Mike Trout' - assert result[1].name == 'Michael Harris' + assert result[0].name == "Mike Trout" + assert result[1].name == "Michael Harris" @pytest.mark.asyncio async def test_search_players_no_season(self, player_service_instance, mock_client): """Test search_players without explicit season.""" - mock_players = [self.create_player_data(1, 'Test Player', pos_1='C')] + mock_players = [self.create_player_data(1, "Test Player", pos_1="C")] - mock_client.get.return_value = { - 'count': 1, - 'players': mock_players - } + mock_client.get.return_value = {"count": 1, "players": mock_players} - result = await player_service_instance.search_players('Test', limit=5) + result = await player_service_instance.search_players("Test", limit=5) - mock_client.get.assert_called_once_with('players/search', params=[('q', 'Test'), ('limit', '5')]) + mock_client.get.assert_called_once_with( + "players/search", params=[("q", "Test"), ("limit", "5")] + ) assert len(result) == 1 - assert result[0].name == 'Test Player' + assert result[0].name == "Test Player" @pytest.mark.asyncio - async def test_search_players_empty_result(self, player_service_instance, mock_client): + async def test_search_players_empty_result( + self, player_service_instance, mock_client + ): """Test search_players with no results.""" mock_client.get.return_value = None - result = await player_service_instance.search_players('NonExistent') + result = await player_service_instance.search_players("NonExistent") - mock_client.get.assert_called_once_with('players/search', params=[('q', 'NonExistent'), ('limit', '10')]) + mock_client.get.assert_called_once_with( + "players/search", params=[("q", "NonExistent"), ("limit", "10")] + ) assert result == [] @pytest.mark.asyncio async def test_search_players_fuzzy(self, player_service_instance, mock_client): - """Test fuzzy search with relevance sorting.""" + """Test fuzzy search delegates to search_players endpoint. + + The fuzzy search now uses the /players/search endpoint which handles + relevance sorting server-side. The limit parameter is passed to the API. + """ mock_data = { - 'count': 3, - 'players': [ - self.create_player_data(1, 'John Smith', team_id=5), # partial match - self.create_player_data(2, 'John', team_id=6), # exact match - self.create_player_data(3, 'Johnny Doe', team_id=7) # partial match - ] + "count": 2, + "total_matches": 3, + "players": [ + self.create_player_data( + 2, "John", team_id=6 + ), # exact match first (API sorts) + self.create_player_data(1, "John Smith", team_id=5), # partial match + ], } mock_client.get.return_value = mock_data - - result = await player_service_instance.search_players_fuzzy('John', limit=2) - - # Should return exact match first, then partial matches, limited to 2 + + result = await player_service_instance.search_players_fuzzy("John", limit=2) + + # Should return results from the search endpoint, limited by API assert len(result) == 2 - assert result[0].name == 'John' # exact match first - mock_client.get.assert_called_once_with('players', params=[('season', '13'), ('name', 'John')]) - + assert result[0].name == "John" # exact match first (sorted by API) + # Now uses the search endpoint instead of get_players_by_name + mock_client.get.assert_called_once_with( + "players/search", params=[("q", "John"), ("limit", "2")] + ) + @pytest.mark.asyncio async def test_get_players_by_position(self, player_service_instance, mock_client): """Test getting players by position.""" mock_data = { - 'count': 2, - 'players': [ - self.create_player_data(1, 'Catcher 1', position='C', team_id=5), - self.create_player_data(2, 'Catcher 2', position='C', team_id=6) - ] + "count": 2, + "players": [ + self.create_player_data(1, "Catcher 1", position="C", team_id=5), + self.create_player_data(2, "Catcher 2", position="C", team_id=6), + ], } mock_client.get.return_value = mock_data - - result = await player_service_instance.get_players_by_position('C', season=12) - + + result = await player_service_instance.get_players_by_position("C", season=12) + assert len(result) == 2 - assert all(p.primary_position == 'C' for p in result) - mock_client.get.assert_called_once_with('players', params=[('position', 'C'), ('season', '12')]) - + assert all(p.primary_position == "C" for p in result) + mock_client.get.assert_called_once_with( + "players", params=[("position", "C"), ("season", "12")] + ) + @pytest.mark.asyncio async def test_error_handling(self, player_service_instance, mock_client): """Test error handling in service methods.""" mock_client.get.side_effect = APIException("API Error") - + # Should return None/empty list on errors, not raise result = await player_service_instance.get_player(1) assert result is None - + result = await player_service_instance.get_players_by_team(5, season=12) assert result == [] class TestPlayerServiceExtras: """Additional coverage tests for PlayerService edge cases.""" - - def create_player_data(self, player_id: int, name: str, team_id: int = 5, position: str = 'C', **kwargs): + + def create_player_data( + self, player_id: int, name: str, team_id: int = 5, position: str = "C", **kwargs + ): """Create complete player data for testing.""" base_data = { - 'id': player_id, - 'name': name, - 'wara': 2.5, - 'season': 12, - 'team_id': team_id, - 'image': f'https://example.com/player{player_id}.jpg', - 'pos_1': position, + "id": player_id, + "name": name, + "wara": 2.5, + "season": 12, + "team_id": team_id, + "image": f"https://example.com/player{player_id}.jpg", + "pos_1": position, } base_data.update(kwargs) return base_data - + @pytest.mark.asyncio async def test_player_service_additional_methods(self): """Test additional PlayerService methods for coverage.""" from services.player_service import PlayerService - + mock_client = AsyncMock() player_service = PlayerService() player_service._client = mock_client - + # Test additional functionality mock_client.get.return_value = { - 'count': 1, - 'players': [self.create_player_data(1, 'Test Player')] + "count": 1, + "players": [self.create_player_data(1, "Test Player")], } - - result = await player_service.get_players_by_name('Test', season=12) + + result = await player_service.get_players_by_name("Test", season=12) assert len(result) == 1 class TestGlobalPlayerServiceInstance: """Test global player service instance.""" - + def test_player_service_global(self): """Test global player service instance.""" assert isinstance(player_service, PlayerService) assert player_service.model_class == Player - assert player_service.endpoint == 'players' - + assert player_service.endpoint == "players" + @pytest.mark.asyncio async def test_service_independence(self): """Test that service instances are independent.""" service1 = PlayerService() service2 = PlayerService() - + # Should be different instances assert service1 is not service2 # But same configuration assert service1.model_class == service2.model_class - assert service1.endpoint == service2.endpoint \ No newline at end of file + assert service1.endpoint == service2.endpoint