Add all-season player search with deduplicated autocomplete
- 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 <noreply@anthropic.com>
This commit is contained in:
Cal Corum
parent
249c17a64c
commit
bf079eab83
@ -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
|
||||
|
||||
@ -54,54 +79,89 @@ class PlayerInfoCommands(commands.Cog):
|
||||
|
||||
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)
|
||||
# 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)
|
||||
self.logger.info("Player search completed", players_found=len(players), season=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
|
||||
|
||||
@ -109,50 +169,76 @@ class PlayerInfoCommands(commands.Cog):
|
||||
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)
|
||||
# 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})" for p in players[:10]])
|
||||
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",
|
||||
self.logger.debug(
|
||||
"Fetching player data and statistics",
|
||||
player_id=player.id,
|
||||
season=search_season)
|
||||
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,
|
||||
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))
|
||||
pitching_stats=bool(pitching_stats),
|
||||
)
|
||||
|
||||
# Create interactive player view with toggleable statistics
|
||||
self.logger.debug("Creating PlayerStatsView with toggleable statistics")
|
||||
@ -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
|
||||
|
||||
@ -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,7 +15,7 @@ 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]):
|
||||
@ -29,9 +30,9 @@ class PlayerService(BaseService[Player]):
|
||||
- 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")
|
||||
|
||||
@ -54,8 +55,9 @@ class PlayerService(BaseService[Player]):
|
||||
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,22 +70,21 @@ 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:
|
||||
@ -102,20 +103,21 @@ class PlayerService(BaseService[Player]):
|
||||
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).
|
||||
|
||||
@ -143,25 +145,37 @@ class PlayerService(BaseService[Player]):
|
||||
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,14 +261,15 @@ 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.
|
||||
@ -232,7 +281,10 @@ class PlayerService(BaseService[Player]):
|
||||
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")
|
||||
@ -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}")
|
||||
@ -286,7 +340,7 @@ class PlayerService(BaseService[Player]):
|
||||
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}")
|
||||
@ -296,7 +350,6 @@ class PlayerService(BaseService[Player]):
|
||||
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}")
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
"""
|
||||
Tests for PlayerService functionality
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
@ -26,16 +27,18 @@ class TestPlayerService:
|
||||
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
|
||||
@ -43,29 +46,31 @@ class TestPlayerService:
|
||||
@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
|
||||
@ -73,22 +78,22 @@ class TestPlayerService:
|
||||
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
|
||||
|
||||
@ -96,81 +101,99 @@ class TestPlayerService:
|
||||
|
||||
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
|
||||
|
||||
@ -178,14 +201,19 @@ class TestPlayerService:
|
||||
|
||||
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)
|
||||
@ -197,86 +225,101 @@ 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)
|
||||
result = await player_service_instance.search_players_fuzzy("John", limit=2)
|
||||
|
||||
# Should return exact match first, then partial matches, limited to 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):
|
||||
@ -294,16 +337,18 @@ class TestPlayerService:
|
||||
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
|
||||
@ -319,11 +364,11 @@ class TestPlayerServiceExtras:
|
||||
|
||||
# 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
|
||||
|
||||
|
||||
@ -334,7 +379,7 @@ class TestGlobalPlayerServiceInstance:
|
||||
"""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):
|
||||
|
||||
Loading…
Reference in New Issue
Block a user