diff --git a/.github/workflows/backend-ci.yml b/.github/workflows/backend-ci.yml index 273077e5..bc8a19c2 100644 --- a/.github/workflows/backend-ci.yml +++ b/.github/workflows/backend-ci.yml @@ -86,7 +86,7 @@ jobs: env: DJANGO_SETTINGS_MODULE: config.settings run: | - pytest lineups/tests.py accounts/tests.py roster/tests.py simulator/tests.py --cov=lineups --cov=accounts --cov=roster --cov=simulator --cov-report=xml --cov-report=term-missing + pytest lineups/tests.py accounts/tests.py roster/tests/ simulator/tests.py --cov=lineups --cov=accounts --cov=roster --cov=simulator --cov-report=xml --cov-report=term-missing - name: Check for security vulnerabilities in dependencies run: | diff --git a/backend/.coveragerc b/backend/.coveragerc index 04c80f2d..aee5e027 100644 --- a/backend/.coveragerc +++ b/backend/.coveragerc @@ -9,6 +9,11 @@ omit = */venv/* */.venv/* */env/* + manage.py + config/asgi.py + config/wsgi.py + */tests_*.py + [report] exclude_lines = diff --git a/backend/config/settings_test.py b/backend/config/settings_test.py index d2a4f1f6..20ff524a 100644 --- a/backend/config/settings_test.py +++ b/backend/config/settings_test.py @@ -5,6 +5,6 @@ DATABASES = { "default": { "ENGINE": "django.db.backends.sqlite3", - "NAME": ":memory:", + "NAME": BASE_DIR / "db.sqlite3", } } diff --git a/backend/pytest.ini b/backend/pytest.ini index 6f65e466..08d211d3 100644 --- a/backend/pytest.ini +++ b/backend/pytest.ini @@ -1,5 +1,5 @@ [pytest] -DJANGO_SETTINGS_MODULE = config.settings +DJANGO_SETTINGS_MODULE = config.settings_test python_files = tests.py test_*.py *_tests.py python_classes = Test* python_functions = test_* @@ -8,7 +8,7 @@ addopts = --strict-markers --tb=short --reuse-db - --ds=config.settings + --ds=config.settings_test --cov=. --cov-report=term-missing --cov-report=html diff --git a/backend/roster/management/commands/import_test_data.py b/backend/roster/management/commands/import_test_data.py index 82ffd405..57ce6567 100644 --- a/backend/roster/management/commands/import_test_data.py +++ b/backend/roster/management/commands/import_test_data.py @@ -1,6 +1,6 @@ from django.core.management.base import BaseCommand, CommandError -from roster.services.importer import import_from_csv +from roster.services.player_import import PlayerImportService class Command(BaseCommand): @@ -30,8 +30,7 @@ def handle(self, *args, **options): dry_run = options.get("dry_run") try: - result = import_from_csv( - path, team_id=team_id_arg, dry_run=dry_run) + result = PlayerImportService.import_from_csv(path, team_id=team_id_arg, dry_run=dry_run) except FileNotFoundError: raise CommandError(f"File not found: {path}") diff --git a/backend/roster/models.py b/backend/roster/models.py index 30857b28..b1ebf4d9 100644 --- a/backend/roster/models.py +++ b/backend/roster/models.py @@ -18,59 +18,58 @@ class Player(models.Model): name = models.CharField(max_length=100, unique=True) created_at = models.DateTimeField(auto_now_add=True) updated_at = models.DateTimeField(auto_now=True) - team = models.ForeignKey( - Team, - on_delete=models.CASCADE, - related_name="players", - null=True, - blank=True, - ) + team = models.ForeignKey(Team, on_delete=models.CASCADE, related_name="players", null=True, blank=True) # Baseball Savant (2025) batter stats snapshot - # Note: These are per-season aggregates - savant_player_id = models.PositiveIntegerField( - null=True, blank=True) # External player_id - year = models.PositiveIntegerField( - null=True, blank=True) # Season year (e.g., 2025) + # Note: These are per-season aggregates; if you later want multi-year history, + # Stat explanations from: https://baseballsavant.mlb.com/leaderboard/custom?year=2025&type=batter&filter=&min=q&selections=pa%2Chome_run%2Ck_percent%2Cbb_percent%2Cslg_percent%2Con_base_percent%2Cisolated_power%2Cr_total_stolen_base%2Cwoba%2Cxwoba%2Cbarrel_batted_rate%2Chard_hit_percent%2Csprint_speed&chart=false&x=pa&y=pa&r=no&chartType=beeswarm&sort=1&sortDir=desc + savant_player_id = models.PositiveIntegerField(null=True, blank=True) # External player_id + year = models.PositiveIntegerField(null=True, blank=True) # Season year (e.g., 2025) ab = models.PositiveIntegerField(null=True, blank=True) # At bat count - pa = models.PositiveIntegerField( - null=True, blank=True) # Plate appearances + pa = models.PositiveIntegerField(null=True, blank=True) # Plate appearances hit = models.PositiveIntegerField(null=True, blank=True) # Total hits single = models.PositiveIntegerField(null=True, blank=True) # Singles double = models.PositiveIntegerField(null=True, blank=True) # Doubles triple = models.PositiveIntegerField(null=True, blank=True) # Triples home_run = models.PositiveIntegerField(null=True, blank=True) # Home runs - strikeout = models.PositiveIntegerField( - null=True, blank=True) # Strikeouts + strikeout = models.PositiveIntegerField(null=True, blank=True) # Strikeouts walk = models.PositiveIntegerField(null=True, blank=True) # Walks - BB - # K%: Frequency of strikeouts per plate appearance - k_percent = models.FloatField(null=True, blank=True) - # BB%: Frequency of walks per plate appearance - bb_percent = models.FloatField(null=True, blank=True) - # SLG: Slugging percentage - total bases per at-bat - slg_percent = models.FloatField(null=True, blank=True) - # OBP: On-base percentage - how often batter reaches base - on_base_percent = models.FloatField(null=True, blank=True) - # ISO: Isolated power - extra bases per at-bat (pure power) - isolated_power = models.FloatField(null=True, blank=True) - # TB: Total bases - b_total_bases = models.FloatField(null=True, blank=True) - # CS: Caught Stealing - r_total_caught_stealing = models.FloatField(null=True, blank=True) - # SB: Stolen bases - r_total_stolen_base = models.FloatField(null=True, blank=True) - # G: Games played - b_game = models.FloatField(null=True, blank=True) - # GIDP: Grounded Into Double Play - b_gnd_into_dp = models.FloatField(null=True, blank=True) - # HBP: Hit by pitch - b_hit_by_pitch = models.FloatField(null=True, blank=True) - # IBB: Intentional walk - b_intent_walk = models.FloatField(null=True, blank=True) - # SH: Sacrifice bunt - b_sac_bunt = models.FloatField(null=True, blank=True) - # SF: Sacrifice fly - b_sac_fly = models.FloatField(null=True, blank=True) + k_percent = models.FloatField(null=True, blank=True) # Frequency of strikeouts per plate appearance - K% = (SO / PA) * 100 + bb_percent = models.FloatField(null=True, blank=True) # Frequency of walks per plate appearance - BB% = (BB / PA) * 100 + slg_percent = models.FloatField( + null=True, blank=True + ) # Measures total bases per at-bat, emphasizes extra-base hits - SLG = (1B + 2*2B + 3*3B + 4*HR) / AB + on_base_percent = models.FloatField( + null=True, blank=True + ) # On-base percentage: Measures how often a batter reaches base safely - OBP = (H + BB + HBP) / (AB + BB + HBP + SF) + isolated_power = models.FloatField( + null=True, blank=True + ) # Shows extra bases per at-bat (pure power) - ISO = SLG - AVG | (AVG = H / AB) + b_total_bases = models.FloatField( + null=True, blank=True + ) # TB: Total bases - 1 x Singles + 2 x Doubles + 3 x Triples + 4 x Home Runs + r_total_caught_stealing = models.FloatField( + null=True, blank=True + ) # CS: Caught Stealing - When a runner is put out by the defense while attempting to steal a base + r_total_stolen_base = models.FloatField( + null=True, blank=True + ) # SB: Stolen bases - Count of successful stolen base attempts + b_game = models.FloatField(null=True, blank=True) # G: Games played - Total games played in a season + b_gnd_into_dp = models.FloatField( + null=True, blank=True + ) # GIPD: Grounded Into Double Play - Counts the times a batter hits a ground ball that results in two outs in one continuous play + b_hit_by_pitch = models.FloatField( + null=True, blank=True + ) # HPB: A hit-by-pitch occurs when a batter is struck by a pitched ball without swinging at it. + b_intent_walk = models.FloatField( + null=True, blank=True + ) # IBB: Intentional Base on Balls / Intentional walk - A strategic play where the defending team deliberately allows a batter to reach first base without having to swing at a pitch. + b_sac_bunt = models.FloatField( + null=True, blank=True + ) # SH: Sacrfice bunt: Occurs when a batter deliberately bunts the ball to advance a baserunner, usually at the cost of being put out themselves + b_sac_fly = models.FloatField( + null=True, blank=True + ) # SF: Sacrifice Fly - Occurs when a batter hits a deep fly ball that is caught by an outfielder (or an infielder playing in the outfield), but a baserunner on third base tags up and scores before the play is over. class Meta: db_table = "players" diff --git a/backend/roster/serializer.py b/backend/roster/serializer.py index 96f815a1..f51d0bf5 100644 --- a/backend/roster/serializer.py +++ b/backend/roster/serializer.py @@ -75,10 +75,8 @@ class Meta(PlayerSerializer.Meta): "id", "name", "team", - "xwoba", "bb_percent", "k_percent", - "barrel_batted_rate", "wos_score", ] diff --git a/backend/roster/services/importer.py b/backend/roster/services/importer.py deleted file mode 100644 index f239d33d..00000000 --- a/backend/roster/services/importer.py +++ /dev/null @@ -1,169 +0,0 @@ -import csv -from decimal import Decimal -from typing import Any, Dict, List, Optional - -from roster.models import Player, Team - - -def _to_float(v): - if v is None or v == "": - return None - try: - return float(Decimal(v)) - except Exception: - try: - return float(str(v).replace("%", "")) - except Exception: - return None - - -def _to_int(v): - if v is None or v == "": - return None - try: - return int(Decimal(v)) - except Exception: - try: - return int(str(v).replace("%", "")) - except Exception: - return None - - -def import_from_csv(path: str, team_id: Optional[int] - = None, dry_run: bool = False) -> Dict[str, Any]: - """Import players from a CSV file into the roster Player model. - - Returns a summary dict with counts and messages. - """ - messages: List[str] = [] - try: - f = open(path, newline="", encoding="utf-8-sig") - except FileNotFoundError: - raise - - reader = csv.DictReader(f) - rows = list(reader) - if not rows: - messages.append("No rows found in CSV.") - f.close() - return {"processed": 0, "created": 0, "updated": 0, "messages": messages} - - if team_id is not None: - Team.objects.get_or_create(pk=team_id) - - processed = created = updated = 0 - - for r in rows: - name = r.get('"last_name, first_name"') or r.get( - "last_name, first_name") or r.get("name") - if name is None: - first = r.get(" first_name") or r.get("first_name") - last = r.get("last_name") - if first and last: - name = f"{last}, {first}" - - if not name: - messages.append(f"Skipping row without name: {r}") - continue - - savant_player_id = r.get("player_id") or r.get("savant_player_id") - year = r.get("year") - pa = r.get("pa") - - # Raw counting stats (for simulation) - hit = r.get("hit") - single = r.get("single") - double = r.get("double") - triple = r.get("triple") - home_run = r.get("home_run") - strikeout = r.get("strikeout") - walk = r.get("walk") - - # Derived percentages - k_percent = _to_float(r.get("k_percent")) - bb_percent = _to_float(r.get("bb_percent")) - slg_percent = _to_float(r.get("slg_percent")) - on_base_percent = _to_float(r.get("on_base_percent")) - isolated_power = _to_float(r.get("isolated_power")) - b_total_bases = _to_int(r.get("b_total_bases")) - r_total_caught_stealing = _to_int(r.get("r_total_caught_stealing")) - r_total_stolen_base = _to_float(r.get("r_total_stolen_base")) - b_game = _to_int(r.get("b_game")) - b_gnd_into_dp = _to_int(r.get("b_gnd_into_dp")) - b_hit_by_pitch = _to_int(r.get("b_hit_by_pitch")) - b_intent_walk = _to_int(r.get("b_intent_walk")) - b_sac_fly = _to_int(r.get("b_sac_fly")) - b_sac_bunt = _to_int(r.get("b_sac_bunt")) - - team_id_csv = None - if "team_id" in r: - team_id_csv = r.get("team_id") - elif '"team_id"' in r: - team_id_csv = r.get('"team_id"') - - use_team_id = None - if team_id_csv: - try: - use_team_id = int(team_id_csv) - except Exception: - use_team_id = None - elif team_id is not None: - use_team_id = team_id - - team_obj = None - if use_team_id is not None: - team_obj, _ = Team.objects.get_or_create(pk=use_team_id) - - defaults: Dict[str, Any] = { - "savant_player_id": ( - int(savant_player_id) - if savant_player_id and str(savant_player_id).isdigit() - else None - ), - "year": int(year) if year and str(year).isdigit() else None, - "pa": int(pa) if pa and str(pa).isdigit() else None, - # Raw counting stats - "hit": int(hit) if hit and str(hit).isdigit() else None, - "single": int(single) if single and str(single).isdigit() else None, - "double": int(double) if double and str(double).isdigit() else None, - "triple": int(triple) if triple and str(triple).isdigit() else None, - "home_run": int(home_run) if home_run and str(home_run).isdigit() else None, - "strikeout": int(strikeout) if strikeout and str(strikeout).isdigit() else None, - "walk": int(walk) if walk and str(walk).isdigit() else None, - # Derived percentages - "k_percent": k_percent, - "bb_percent": bb_percent, - "slg_percent": slg_percent, - "on_base_percent": on_base_percent, - "isolated_power": isolated_power, - "b_total_bases": b_total_bases, - "r_total_caught_stealing": r_total_caught_stealing, - "r_total_stolen_base": r_total_stolen_base, - "b_game": b_game, - "b_gnd_into_dp": b_gnd_into_dp, - "b_hit_by_pitch": b_hit_by_pitch, - "b_intent_walk": b_intent_walk, - "b_sac_fly": b_sac_fly, - "b_sac_bunt": b_sac_bunt, - } - - if dry_run: - messages.append( - f"Would import player: {name} team_id={use_team_id} fields={defaults}") - processed += 1 - continue - - player, created_flag = Player.objects.update_or_create( - name=name, - defaults={**defaults, "team": team_obj}, - ) - processed += 1 - if created_flag: - created += 1 - else: - updated += 1 - - f.close() - messages.append( - f"Processed {processed} rows: created={created} updated={updated}") - return {"processed": processed, "created": created, "updated": updated, "messages": messages} diff --git a/backend/roster/services/load_csv.py b/backend/roster/services/load_csv.py deleted file mode 100644 index 2cc9efa4..00000000 --- a/backend/roster/services/load_csv.py +++ /dev/null @@ -1,119 +0,0 @@ -"""Service function to import batter stats CSV into the roster Player model. - -This was converted from the previous management command to a callable -service function so it can be invoked from code, tests or a lightweight -management wrapper if desired. -""" - -import csv -from pathlib import Path -from typing import Tuple - -from django.db import transaction - -from roster.models import Player - - -def load_csv_file(file: str | Path, year: int | None = None, - dry_run: bool = False) -> Tuple[int, int, int]: - """Load CSV into roster.Player. - - Returns a tuple (imported, updated, skipped). - """ - file_path = Path(file) - if not file_path.exists(): - raise FileNotFoundError(f"CSV file not found: {file_path}") - - imported = 0 - updated = 0 - skipped = 0 - - with file_path.open(newline="", encoding="utf-8-sig") as fh: - reader = csv.DictReader(fh) - rows = list(reader) - - if dry_run: - # Caller can print or assert the content as needed - return (0, 0, len(rows)) - - with transaction.atomic(): - for r in rows: - raw_name = r.get("last_name, first_name") - if not raw_name: - skipped += 1 - continue - - name = raw_name.strip() - - player, created = Player.objects.get_or_create(name=name) - - def fflt(key: str): - v = r.get(key) - if v is None or v == "": - return None - try: - return float(str(v).replace('"', "").replace("%", "").strip()) - except ValueError: - return None - - def to_int_cell(key: str): - v = r.get(key) - if v is None or v == "": - return None - try: - return int(float(str(v).replace('"', "").replace("%", "").strip())) - except ValueError: - return None - - try: - savant_id = int(r.get("player_id")) if r.get( - "player_id") else None - except ValueError: - savant_id = None - - try: - pa = int(r.get("pa")) if r.get("pa") else None - except ValueError: - pa = None - - row_year = int(r.get("year")) if r.get("year") else None - use_year = year if year is not None else row_year - - player.savant_player_id = savant_id - player.year = use_year - player.ab = to_int_cell("ab") - player.pa = pa - player.hit = to_int_cell("hit") - player.single = to_int_cell("single") - player.double = to_int_cell("double") - player.triple = to_int_cell("triple") - player.home_run = to_int_cell("home_run") - player.walk = to_int_cell("walk") - player.k_percent = fflt("k_percent") - player.bb_percent = fflt("bb_percent") - player.slg_percent = fflt("slg_percent") - player.on_base_percent = fflt("on_base_percent") - player.isolated_power = fflt("isolated_power") - player.b_total_bases = to_int_cell("b_total_bases") - player.r_total_caught_stealing = to_int_cell( - "r_total_caught_stealing") - player.r_total_stolen_base = to_int_cell("r_total_stolen_base") - player.b_game = fflt("b_game") - player.b_gnd_into_dp = fflt("b_gnd_into_dp") - player.b_hit_by_pitch = fflt("b_hit_by_pitch") - player.b_intent_walk = fflt("b_intent_walk") - player.b_sac_fly = fflt("b_sac_fly") - player.b_total_sacrifices = fflt("b_total_sacrifices") - player.woba = fflt("woba") - player.xwoba = fflt("xwoba") - player.barrel_batted_rate = fflt("barrel_batted_rate") - player.hard_hit_percent = fflt("hard_hit_percent") - player.sprint_speed = fflt("sprint_speed") - player.save() - - if created: - imported += 1 - else: - updated += 1 - - return (imported, updated, skipped) diff --git a/backend/roster/services/player_import.py b/backend/roster/services/player_import.py new file mode 100644 index 00000000..289a6bc7 --- /dev/null +++ b/backend/roster/services/player_import.py @@ -0,0 +1,207 @@ +import csv +from decimal import Decimal +from pathlib import Path +from typing import Any, Dict, List, Optional + +from django.db import transaction + +from roster.models import Player, Team + + +class PlayerImportService: + """Service for importing players from CSV files.""" + + @staticmethod + def import_from_csv( + path: str | Path, team_id: Optional[int] = None, dry_run: bool = False + ) -> Dict[str, Any]: + """ + Import players from a CSV file into the roster Player model. + + Args: + path: Path to the CSV file + team_id: Optional team ID to assign to all players + dry_run: If True, simulate import without saving + + Returns: + Dict containing import summary (processed, created, updated, messages) + """ + messages: List[str] = [] + file_path = Path(path) + + if not file_path.exists(): + raise FileNotFoundError(f"CSV file not found: {file_path}") + + try: + with file_path.open(newline="", encoding="utf-8-sig") as f: + reader = csv.DictReader(f) + rows = list(reader) + except Exception as e: + raise ValueError(f"Error reading CSV: {e}") + + if not rows: + return { + "processed": 0, + "created": 0, + "updated": 0, + "messages": ["No rows found in CSV."], + } + + # Pre-fetch or create team if provided globally + global_team = None + if team_id is not None: + global_team, _ = Team.objects.get_or_create(pk=team_id) + + processed = 0 + created_count = 0 + updated_count = 0 + + # Use transaction for data integrity (unless dry_run) + try: + with transaction.atomic(): + for r in rows: + # 1. Extract Name + name = PlayerImportService._extract_name(r) + if not name: + messages.append(f"Skipping row without name: {r}") + continue + + # 2. Determine Team + team_obj = PlayerImportService._determine_team(r, global_team) + + # 3. Prepare Data + defaults = PlayerImportService._prepare_player_data(r) + + if dry_run: + messages.append( + f"Would import player: {name} team={team_obj.id if team_obj else 'None'} fields={defaults}" + ) + processed += 1 + continue + + # 4. Update or Create + player, created = Player.objects.update_or_create( + name=name, + defaults={**defaults, "team": team_obj}, + ) + + processed += 1 + if created: + created_count += 1 + else: + updated_count += 1 + + if dry_run: + # Rollback is implicit since we didn't write, but good to be explicit mentally + pass + + except Exception as e: + if not dry_run: + messages.append(f"Import failed: {e}") + raise e + + messages.append( + f"Processed {processed} rows: created={created_count} updated={updated_count}" + ) + return { + "processed": processed, + "created": created_count, + "updated": updated_count, + "messages": messages, + } + + @staticmethod + def _extract_name(row: Dict[str, Any]) -> Optional[str]: + """Extract and normalize player name from row.""" + name = ( + row.get('"last_name, first_name"') + or row.get("last_name, first_name") + or row.get("name") + ) + + # If name is None or empty string, try to construct from parts + if not name: + first = row.get(" first_name") or row.get("first_name") + last = row.get("last_name") + if first and last: + name = f"{last}, {first}" + + return name.strip() if name else None + + @staticmethod + def _determine_team(row: Dict[str, Any], global_team: Optional[Team]) -> Optional[Team]: + """Determine team for the player.""" + team_id_csv = None + if "team_id" in row: + team_id_csv = row.get("team_id") + elif '"team_id"' in row: + team_id_csv = row.get('"team_id"') + + if team_id_csv: + try: + tid = int(team_id_csv) + team, _ = Team.objects.get_or_create(pk=tid) + return team + except Exception: + pass + + return global_team + + @staticmethod + def _prepare_player_data(r: Dict[str, Any]) -> Dict[str, Any]: + """Parse and convert CSV row data into model fields.""" + + def _to_float(v): + if v is None or v == "": + return None + try: + return float(Decimal(v)) + except Exception: + try: + return float(str(v).replace("%", "").replace('"', "")) + except Exception: + return None + + def _to_int(v): + if v is None or v == "": + return None + try: + return int(Decimal(v)) + except Exception: + try: + return int(float(str(v).replace("%", "").replace('"', ""))) + except Exception: + return None + + # Helper to get value from row with fallback + def get_val(key): + return r.get(key) + + return { + "savant_player_id": _to_int(get_val("player_id") or get_val("savant_player_id")), + "year": _to_int(get_val("year")), + "pa": _to_int(get_val("pa")), + # Raw counting stats + "hit": _to_int(get_val("hit")), + "single": _to_int(get_val("single")), + "double": _to_int(get_val("double")), + "triple": _to_int(get_val("triple")), + "home_run": _to_int(get_val("home_run")), + "strikeout": _to_int(get_val("strikeout")), + "walk": _to_int(get_val("walk")), + # Derived percentages + "k_percent": _to_float(get_val("k_percent")), + "bb_percent": _to_float(get_val("bb_percent")), + "slg_percent": _to_float(get_val("slg_percent")), + "on_base_percent": _to_float(get_val("on_base_percent")), + "isolated_power": _to_float(get_val("isolated_power")), + "b_total_bases": _to_int(get_val("b_total_bases")), + "r_total_caught_stealing": _to_int(get_val("r_total_caught_stealing")), + "r_total_stolen_base": _to_float(get_val("r_total_stolen_base")), + "b_game": _to_int(get_val("b_game")), + "b_gnd_into_dp": _to_int(get_val("b_gnd_into_dp")), + "b_hit_by_pitch": _to_int(get_val("b_hit_by_pitch")), + "b_intent_walk": _to_int(get_val("b_intent_walk")), + "b_sac_fly": _to_int(get_val("b_sac_fly")), + "b_sac_bunt": _to_int(get_val("b_sac_bunt")), + } diff --git a/backend/roster/services/player_ranking.py b/backend/roster/services/player_ranking.py index ca66007d..a19babe8 100644 --- a/backend/roster/services/player_ranking.py +++ b/backend/roster/services/player_ranking.py @@ -11,6 +11,9 @@ from roster.models import Player, Team + + + def get_all_players_with_stats() -> List[Dict[str, Any]]: """ Fetch all players with a subset of their stats as dictionaries. @@ -30,8 +33,7 @@ def get_all_players_with_stats() -> List[Dict[str, Any]]: ) -def get_ranked_players(ascending: bool = False, - top_n: Optional[int] = None) -> List[Dict[str, Any]]: +def get_ranked_players(ascending: bool = False, top_n: Optional[int] = None) -> List[Dict[str, Any]]: """ Placeholder ranking: returns all players with a synthetic `wos_score` field. """ @@ -54,39 +56,6 @@ def get_ranked_players(ascending: bool = False, return result -# def get_ranked_players(ascending=False, top_n=None) -> List[Dict[str, Any]]: -# """ -# Get players ranked by WOS score. - -# TODO: Implement actual WOS ranking algorithm. -# For now, returns all players without ranking. - -# Args: -# ascending: Sort order (False = highest first) -# top_n: Optional limit on number of results - -# Returns: -# List of player dictionaries with wos_score field added. -# """ -# players = get_all_players_with_stats() - -# if not players: -# return [] - -# # TODO: Replace with actual WOS ranking logic from lineups/services/algorithm_logic.py -# result = [] -# for player in players: -# player_data = dict(player) -# player_data["wos_score"] = 0.0 # Placeholder -# result.append(player_data) - -# # Apply limit if specified -# if top_n: -# result = result[:top_n] - -# return result - - def create_player_with_stats(name: str, **stats) -> Player: """ Create a new player with optional stats. diff --git a/backend/roster/tests.py b/backend/roster/tests.py deleted file mode 100644 index cd20a944..00000000 --- a/backend/roster/tests.py +++ /dev/null @@ -1,118 +0,0 @@ -from django.test import TestCase -from django.urls import reverse -from rest_framework import status -from rest_framework.test import APITestCase - -from .models import Team -from .services.player_ranking import get_ranked_players - -# @pytest.mark.skip(reason="Requires database migrations with home_run column") -# class PlayerModelTests(TestCase): -# """Test Player model functionality.""" -# -# def setUp(self): -# self.team = Team.objects.create(name="Test Team") -# -# def test_create_player(self): -# """Test creating a player.""" -# player = Player.objects.create(name="Test Player", team=self.team, position="SS") -# self.assertEqual(player.name, "Test Player") -# self.assertEqual(player.team, self.team) -# -# def test_player_str(self): -# """Test player string representation.""" -# player = Player.objects.create(name="John Doe", position="CF") -# self.assertEqual(str(player), "John Doe (CF)") - - -class PlayerRankingServiceTests(TestCase): - """Test player ranking service functions.""" - - def test_get_ranked_players_empty(self): - """Test ranking with no players.""" - result = get_ranked_players() - self.assertEqual(result, []) - - -# @pytest.mark.skip(reason="Requires database migrations with home_run column") -# class PlayerAPITests(APITestCase): -# """Test Player API endpoints.""" -# -# def setUp(self): -# self.team = Team.objects.create(name="Yankees") -# self.player = Player.objects.create( -# name="Aaron Judge", -# team=self.team, -# position="RF", -# xwoba=0.476, -# bb_percent=18.3, -# barrel_batted_rate=24.7, -# k_percent=23.6, -# ) -# -# def test_list_players(self): -# """Test GET /players/""" -# url = reverse("roster:player-list") -# response = self.client.get(url) -# self.assertEqual(response.status_code, status.HTTP_200_OK) -# self.assertEqual(len(response.data), 1) -# -# def test_create_player(self): -# """Test POST /players/""" -# url = reverse("roster:player-list") -# data = {"name": "Juan Soto", "team": self.team.id, "position": "RF"} -# response = self.client.post(url, data) -# self.assertEqual(response.status_code, status.HTTP_201_CREATED) -# self.assertEqual(Player.objects.count(), 2) -# -# def test_retrieve_player(self): -# """Test GET /players/{id}/""" -# url = reverse("roster:player-detail", kwargs={"pk": self.player.id}) -# response = self.client.get(url) -# self.assertEqual(response.status_code, status.HTTP_200_OK) -# self.assertEqual(response.data["name"], "Aaron Judge") -# -# def test_update_player(self): -# """Test PATCH /players/{id}/""" -# url = reverse("roster:player-detail", kwargs={"pk": self.player.id}) -# data = {"position": "CF"} -# response = self.client.patch(url, data) -# self.assertEqual(response.status_code, status.HTTP_200_OK) -# self.player.refresh_from_db() -# self.assertEqual(self.player.position, "CF") -# -# def test_delete_player(self): -# """Test DELETE /players/{id}/""" -# url = reverse("roster:player-detail", kwargs={"pk": self.player.id}) -# response = self.client.delete(url) -# self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) -# self.assertEqual(Player.objects.count(), 0) -# -# def test_ranked_players(self): -# """Test GET /players/ranked/""" -# url = reverse("roster:player-ranked") -# response = self.client.get(url) -# self.assertEqual(response.status_code, status.HTTP_200_OK) -# self.assertIn("players", response.data) -# self.assertEqual(len(response.data["players"]), 1) -# self.assertIn("wos_score", response.data["players"][0]) - - -class TeamAPITests(APITestCase): - """Test Team API endpoints.""" - - def test_list_teams(self): - """Test GET /teams/""" - Team.objects.create() - url = reverse("roster:team-list") - response = self.client.get(url) - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(len(response.data), 1) - - def test_create_team(self): - """Test POST /teams/""" - url = reverse("roster:team-list") - data = {} - response = self.client.post(url, data) - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(Team.objects.count(), 1) diff --git a/backend/roster/tests/__init__.py b/backend/roster/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/backend/roster/tests/test_api.py b/backend/roster/tests/test_api.py new file mode 100644 index 00000000..39d8768f --- /dev/null +++ b/backend/roster/tests/test_api.py @@ -0,0 +1,83 @@ +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APITestCase +from roster.models import Player, Team + +class PlayerAPITests(APITestCase): + """Test Player API endpoints.""" + + def setUp(self): + self.team = Team.objects.create(id=1) + self.player = Player.objects.create( + name="Aaron Judge", + team=self.team, + bb_percent=18.3, + k_percent=23.6, + ) + + def test_list_players(self): + """Test GET /players/""" + url = reverse("roster:player-list") + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.data), 1) + + def test_create_player(self): + """Test POST /players/""" + url = reverse("roster:player-list") + data = {"name": "Juan Soto", "team": self.team.id, "home_run": 40} + response = self.client.post(url, data) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(Player.objects.count(), 2) + + def test_retrieve_player(self): + """Test GET /players/{id}/""" + url = reverse("roster:player-detail", kwargs={"pk": self.player.id}) + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["name"], "Aaron Judge") + + def test_update_player(self): + """Test PATCH /players/{id}/""" + url = reverse("roster:player-detail", kwargs={"pk": self.player.id}) + data = {"home_run": 62} + response = self.client.patch(url, data) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.player.refresh_from_db() + self.assertEqual(self.player.home_run, 62) + + def test_delete_player(self): + """Test DELETE /players/{id}/""" + url = reverse("roster:player-detail", kwargs={"pk": self.player.id}) + response = self.client.delete(url) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + self.assertEqual(Player.objects.count(), 0) + + def test_ranked_players(self): + """Test GET /players/ranked/""" + url = reverse("roster:player-ranked") + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIn("players", response.data) + self.assertEqual(len(response.data["players"]), 1) + self.assertIn("wos_score", response.data["players"][0]) + + +class TeamAPITests(APITestCase): + """Test Team API endpoints.""" + + def test_list_teams(self): + """Test GET /teams/""" + Team.objects.create(id=1) + url = reverse("roster:team-list") + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.data), 1) + + def test_create_team(self): + """Test POST /teams/""" + url = reverse("roster:team-list") + data = {} + response = self.client.post(url, data) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(Team.objects.count(), 1) diff --git a/backend/roster/tests/test_csv_import.py b/backend/roster/tests/test_csv_import.py new file mode 100644 index 00000000..f6ac3bf0 --- /dev/null +++ b/backend/roster/tests/test_csv_import.py @@ -0,0 +1,37 @@ +import os +from pathlib import Path +from django.test import TestCase +from django.conf import settings +from roster.models import Player, Team +from roster.services.player_import import PlayerImportService + +class TestDataImportTestCase(TestCase): + """Test importing the actual test_data.csv file.""" + + def test_import_real_csv(self): + """Verify that data/test_data.csv can be imported successfully.""" + # Locate the CSV file relative to the backend directory + # Assuming backend is at project_root/backend + # and data is at project_root/data + base_dir = settings.BASE_DIR.parent # This should be project_root + csv_path = base_dir / "data" / "test_data.csv" + + if not csv_path.exists(): + # Fallback for different environment structures + csv_path = Path("..") / "data" / "test_data.csv" + + if not csv_path.exists(): + self.fail(f"test_data.csv not found at {csv_path.resolve()}") + + print(f"Importing from {csv_path.resolve()}") + + result = PlayerImportService.import_from_csv(csv_path) + + self.assertGreater(result["processed"], 0, "Should process at least one row") + self.assertGreater(result["created"], 0, "Should create at least one player") + + # Verify some data integrity if possible + # e.g. check if a known player exists + # Since I don't know the exact content, I'll just check counts + self.assertTrue(Player.objects.exists()) + self.assertTrue(Team.objects.exists()) diff --git a/backend/roster/tests/test_models.py b/backend/roster/tests/test_models.py new file mode 100644 index 00000000..546ad9d9 --- /dev/null +++ b/backend/roster/tests/test_models.py @@ -0,0 +1,20 @@ +from django.test import TestCase +from roster.models import Player, Team + +class PlayerModelTests(TestCase): + """Test Player model functionality.""" + + def setUp(self): + self.team = Team.objects.create(id=1) + + def test_create_player(self): + """Test creating a player.""" + player = Player.objects.create(name="Test Player", team=self.team, home_run=10) + self.assertEqual(player.name, "Test Player") + self.assertEqual(player.team, self.team) + self.assertEqual(player.home_run, 10) + + def test_player_str(self): + """Test player string representation.""" + player = Player.objects.create(name="John Doe", team=self.team) + self.assertEqual(str(player), "John Doe") diff --git a/backend/roster/tests/test_player_import.py b/backend/roster/tests/test_player_import.py new file mode 100644 index 00000000..2311c635 --- /dev/null +++ b/backend/roster/tests/test_player_import.py @@ -0,0 +1,213 @@ +import csv +import tempfile +from pathlib import Path +from unittest.mock import patch + +from django.test import TestCase + +from roster.models import Player, Team +from roster.services.player_import import PlayerImportService + + +class PlayerImportServiceTestCase(TestCase): + """Test PlayerImportService functionality.""" + + def setUp(self): + self.team = Team.objects.create(id=1) + self.temp_dir = tempfile.TemporaryDirectory() + self.csv_path = Path(self.temp_dir.name) / "test_players.csv" + + def tearDown(self): + self.temp_dir.cleanup() + + def create_csv(self, rows): + """Helper to create a CSV file with given rows.""" + with open(self.csv_path, "w", newline="", encoding="utf-8-sig") as f: + writer = csv.DictWriter(f, fieldnames=rows[0].keys()) + writer.writeheader() + writer.writerows(rows) + return str(self.csv_path) + + def test_import_from_csv_success(self): + """Test successful import of valid player data.""" + rows = [ + { + "last_name, first_name": "Judge, Aaron", + "player_id": "12345", + "year": "2024", + "pa": "600", + "hit": "150", + "home_run": "50", + "team_id": str(self.team.id), + }, + { + "last_name, first_name": "Soto, Juan", + "player_id": "67890", + "year": "2024", + "pa": "650", + "hit": "160", + "home_run": "40", + # No team_id, should use global or None + }, + ] + self.create_csv(rows) + + result = PlayerImportService.import_from_csv(self.csv_path, team_id=self.team.id) + + self.assertEqual(result["created"], 2) + self.assertEqual(Player.objects.count(), 2) + + judge = Player.objects.get(savant_player_id=12345) + self.assertEqual(judge.name, "Judge, Aaron") + self.assertEqual(judge.home_run, 50) + self.assertEqual(judge.team, self.team) + + soto = Player.objects.get(savant_player_id=67890) + self.assertEqual(soto.name, "Soto, Juan") + self.assertEqual(soto.team, self.team) # Fallback to global team + + def test_import_updates_existing_player(self): + """Test that importing updates existing players instead of creating duplicates.""" + # Create initial player + Player.objects.create( + name="Judge, Aaron", + savant_player_id=12345, + team=self.team, + home_run=10 + ) + + rows = [ + { + "last_name, first_name": "Judge, Aaron", + "player_id": "12345", + "year": "2024", + "pa": "600", + "hit": "150", + "home_run": "62", # Updated value + "team_id": str(self.team.id), + } + ] + self.create_csv(rows) + + result = PlayerImportService.import_from_csv(self.csv_path) + + self.assertEqual(result["created"], 0) + self.assertEqual(result["updated"], 1) + self.assertEqual(Player.objects.count(), 1) + + judge = Player.objects.get(name="Judge, Aaron") + self.assertEqual(judge.home_run, 62) + + def test_import_dry_run(self): + """Test dry run mode does not save changes.""" + rows = [ + { + "last_name, first_name": "New Player", + "player_id": "99999", + "year": "2024", + "pa": "100", + } + ] + self.create_csv(rows) + + result = PlayerImportService.import_from_csv(self.csv_path, dry_run=True) + + self.assertEqual(result["processed"], 1) + self.assertEqual(Player.objects.count(), 0) # Nothing saved + + def test_file_not_found(self): + """Test error when file does not exist.""" + with self.assertRaises(FileNotFoundError): + PlayerImportService.import_from_csv("non_existent.csv") + + def test_empty_file(self): + """Test importing an empty file.""" + # Create empty file + with open(self.csv_path, "w") as f: + pass + + # Should fail or return empty result depending on implementation + # The current implementation catches Exception on DictReader if empty? + # Actually DictReader on empty file yields no rows. + result = PlayerImportService.import_from_csv(self.csv_path) + self.assertEqual(result["processed"], 0) + + def test_name_parsing_formats(self): + """Test various name formats.""" + rows = [ + {"last_name, first_name": "Doe, John", "player_id": "1"}, + {"name": "Smith, Jane", "player_id": "2"}, + {"first_name": "Bob", "last_name": "Jones", "player_id": "3"}, + ] + # We need to write a header that includes all possible keys to avoid DictReader mismatch if we want strictness, + # but DictReader just reads what's there. + # Let's write them carefully. + with open(self.csv_path, "w", newline="", encoding="utf-8-sig") as f: + writer = csv.DictWriter(f, fieldnames=["last_name, first_name", "name", "first_name", "last_name", "player_id", "year", "pa"]) + writer.writeheader() + writer.writerows(rows) + + result = PlayerImportService.import_from_csv(self.csv_path) + self.assertEqual(result["created"], 3) + + self.assertTrue(Player.objects.filter(name="Doe, John").exists()) + self.assertTrue(Player.objects.filter(name="Smith, Jane").exists()) + self.assertTrue(Player.objects.filter(name="Jones, Bob").exists()) + + def test_data_conversion_robustness(self): + """Test that invalid numbers are handled gracefully (converted to None or 0).""" + rows = [ + { + "last_name, first_name": "Robusto", + "player_id": "100", + "pa": "invalid", # Should be None + "hit": "10%", # Should strip % + "bb_percent": "12.5", + } + ] + self.create_csv(rows) + + PlayerImportService.import_from_csv(self.csv_path) + player = Player.objects.get(name="Robusto") + + def test_import_csv_read_error(self): + """Test error handling when reading CSV fails.""" + # Create a file that exists so we pass the exists() check + rows = [{"name": "Test"}] + self.create_csv(rows) + + # Mock the open method on Path instances to raise an error + with patch("roster.services.player_import.Path.open", side_effect=IOError("Read error")): + with self.assertRaises(ValueError) as cm: + PlayerImportService.import_from_csv(self.csv_path) + self.assertIn("Error reading CSV", str(cm.exception)) + + def test_import_row_without_name(self): + """Test that rows without a valid name are skipped.""" + rows = [ + { + "player_id": "111", + "year": "2024", + # No name fields + } + ] + self.create_csv(rows) + + result = PlayerImportService.import_from_csv(self.csv_path) + + self.assertEqual(result["processed"], 0) + self.assertEqual(result["created"], 0) + self.assertTrue(any("Skipping row without name" in msg for msg in result["messages"])) + + def test_import_transaction_error(self): + """Test error handling when a database error occurs during import.""" + rows = [ + {"name": "Error Player", "player_id": "999"} + ] + self.create_csv(rows) + + # Mock Player.objects.update_or_create to raise an exception + with patch("roster.models.Player.objects.update_or_create", side_effect=Exception("DB Error")): + with self.assertRaises(Exception) as cm: + PlayerImportService.import_from_csv(self.csv_path) + self.assertIn("DB Error", str(cm.exception)) diff --git a/backend/roster/tests/test_player_ranking.py b/backend/roster/tests/test_player_ranking.py new file mode 100644 index 00000000..9b6d1b5e --- /dev/null +++ b/backend/roster/tests/test_player_ranking.py @@ -0,0 +1,63 @@ +from django.test import TestCase + +from roster.models import Player, Team +from roster.services.player_ranking import ( + create_player_with_stats, + get_ranked_players, + get_team_by_id, + update_player_stats, +) + + +class PlayerRankingServiceTestCase(TestCase): + """Test PlayerRankingService and helper functions.""" + + def setUp(self): + self.team = Team.objects.create(id=1) + # Create players with different stats + self.p1 = Player.objects.create(name="Player 1", team=self.team, bb_percent=10.0, pa=100) + self.p2 = Player.objects.create(name="Player 2", team=self.team, bb_percent=5.0, pa=100) + self.p3 = Player.objects.create(name="Player 3", team=self.team, bb_percent=15.0, pa=100) + + def test_get_ranked_players(self): + """Test fetching ranked players (placeholder logic).""" + # The current placeholder implementation just returns players with wos_score=0 + # We verify it returns the correct structure + ranked = get_ranked_players() + self.assertEqual(len(ranked), 3) + self.assertIn("wos_score", ranked[0]) + self.assertEqual(ranked[0]["wos_score"], 0.0) + + def test_get_ranked_players_top_n(self): + """Test fetching ranked players with top_n limit.""" + ranked = get_ranked_players(top_n=2) + self.assertEqual(len(ranked), 2) + + def test_create_player_with_stats(self): + """Test helper to create player.""" + p = create_player_with_stats("New Guy", bb_percent=12.5, team=self.team) + self.assertEqual(p.name, "New Guy") + self.assertEqual(p.bb_percent, 12.5) + self.assertEqual(p.team, self.team) + + def test_update_player_stats(self): + """Test helper to update player.""" + p = update_player_stats(self.p1.id, bb_percent=20.0) + + self.p1.refresh_from_db() + self.assertEqual(self.p1.bb_percent, 20.0) + + def test_get_team_by_id(self): + """Test helper to get team.""" + t = get_team_by_id(self.team.id) + self.assertEqual(t, self.team) + + t_none = get_team_by_id(999) + self.assertIsNone(t_none) + + def test_get_ranked_players_empty(self): + """Test ranking with no players.""" + # Clear players created in setUp + Player.objects.all().delete() + result = get_ranked_players() + self.assertEqual(result, []) diff --git a/backend/roster/tests/test_serializers.py b/backend/roster/tests/test_serializers.py new file mode 100644 index 00000000..badd1d53 --- /dev/null +++ b/backend/roster/tests/test_serializers.py @@ -0,0 +1,86 @@ +from django.test import TestCase +from rest_framework.exceptions import ValidationError + +from roster.models import Player, Team +from roster.serializer import ( + PlayerCreateSerializer, + PlayerPartialUpdateSerializer, + PlayerSerializer, +) + + +class PlayerSerializerTestCase(TestCase): + """Test PlayerSerializer validation.""" + + def setUp(self): + self.team = Team.objects.create(id=1) + + def test_player_create_serializer_valid_name(self): + """Test creating a player with valid name.""" + data = { + "name": "Test Player", + "team": self.team.id, + "pa": 100, + } + serializer = PlayerCreateSerializer(data=data) + self.assertTrue(serializer.is_valid()) + + def test_player_create_serializer_empty_name(self): + """Test that empty player name raises validation error.""" + data = { + "name": " ", # Only whitespace + "team": self.team.id, + "pa": 100, + } + serializer = PlayerCreateSerializer(data=data) + self.assertFalse(serializer.is_valid()) + self.assertIn("name", serializer.errors) + + def test_player_partial_update_serializer_empty_name(self): + """Test that empty name in partial update raises validation error.""" + player = Player.objects.create(name="Original Name", team=self.team, pa=100) + + data = {"name": " "} # Only whitespace + serializer = PlayerPartialUpdateSerializer(player, data=data, partial=True) + + self.assertFalse(serializer.is_valid()) + self.assertIn("name", serializer.errors) + + def test_player_partial_update_serializer_none_name(self): + """Test that None name in partial update is handled correctly.""" + player = Player.objects.create(name="Original Name", team=self.team, pa=100) + + # Partial update without name field should work + data = {"pa": 200} + serializer = PlayerPartialUpdateSerializer(player, data=data, partial=True) + + self.assertTrue(serializer.is_valid()) + + def test_mixin_validate_name_direct(self): + """Test PlayerNameValidationMixin.validate_name directly.""" + from roster.serializer import PlayerNameValidationMixin + mixin = PlayerNameValidationMixin() + + # Test valid name + self.assertEqual(mixin.validate_name(" Valid Name "), "Valid Name") + + # Test empty name raises ValidationError + with self.assertRaises(ValidationError): + mixin.validate_name(" ") + + def test_partial_update_validate_name_direct(self): + """Test PlayerPartialUpdateSerializer.validate_name directly.""" + serializer = PlayerPartialUpdateSerializer() + + # Test valid name + self.assertEqual(serializer.validate_name(" Valid Name "), "Valid Name") + + # Test empty name raises ValidationError + with self.assertRaises(ValidationError): + serializer.validate_name(" ") + + # Test None/empty value returns as is (if logic allows, though validate_name usually gets value) + # The code says: if value and not value.strip(): raise + # So if value is None, it returns None + self.assertIsNone(serializer.validate_name(None)) + self.assertEqual(serializer.validate_name(""), "") diff --git a/backend/roster/urls.py b/backend/roster/urls.py index 04770037..57a5cf52 100644 --- a/backend/roster/urls.py +++ b/backend/roster/urls.py @@ -22,11 +22,4 @@ urlpatterns = [ path("", include(router.urls)), - path("sort-by-woba/", views.sort_players_by_woba, name="sort-by-woba"), - # Public: check if sample players are loaded - path("sample-players-status/", views.check_sample_players_status, - name="sample-players-status"), - # Admin only: load sample players from CSV - path("load-sample-players/", views.load_sample_players, - name="load-sample-players"), ] diff --git a/backend/roster/views.py b/backend/roster/views.py index 72340d00..e0b0eaab 100644 --- a/backend/roster/views.py +++ b/backend/roster/views.py @@ -3,39 +3,15 @@ from django.http import JsonResponse from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_http_methods -from rest_framework import viewsets -from rest_framework.decorators import api_view, permission_classes -from rest_framework.permissions import IsAdminUser +from rest_framework import status, viewsets +from rest_framework.decorators import action +from rest_framework.response import Response from .models import Player, Team from .serializer import PlayerSerializer, TeamSerializer -from .services.sample_data_loader import load_sample_players as load_sample_data -# TODO: implement post as well for clarity -@csrf_exempt -@require_http_methods(["GET", "POST"]) -def players(request): - """API endpoint to list and create players.""" - if request.method == "GET": - # List all players with stats - players = Player.objects.all().values( - "id", - "name", - "xwoba", - "bb_percent", - "k_percent", - "barrel_batted_rate", - "pa", - "year", - "created_at", - "updated_at", - ) - player_data = list(players) - return JsonResponse({"players": player_data}) - -# TODO: what does this do is it needed clarify? class TeamViewSet(viewsets.ModelViewSet): """ ViewSet for Team model. @@ -56,108 +32,11 @@ class PlayerViewSet(viewsets.ModelViewSet): serializer_class = PlayerSerializer -@csrf_exempt -@require_http_methods(["DELETE"]) -def player_detail(request, player_id): - """API endpoint to delete a specific player.""" - try: - player = Player.objects.get(id=player_id) - player_name = player.name - player.delete() - return JsonResponse( - {"message": f"Player '{player_name}' deleted successfully"} - ) - except Player.DoesNotExist: - return JsonResponse({"error": "Player not found"}, status=404) - except Exception as e: - return JsonResponse({"error": str(e)}, status=400) - - -# TODO: get rid of old method -@csrf_exempt -@require_http_methods(["GET"]) -def players_ranked(request): - """API endpoint to get players ranked by WOS score.""" - try: - # TODO: Implement actual WOS ranking logic - # For now, returns empty list as placeholder - result = [] - - return JsonResponse({"players": result}) - - except Exception as e: - return JsonResponse({"error": str(e)}, status=500) - - -@csrf_exempt -@require_http_methods(["POST"]) -def sort_players_by_woba(request): - """API endpoint to sort a list of player IDs by wOBA (descending). - - Request body should be JSON: {"player_ids": [1, 2, 3, ...]} - Returns: {"player_ids": [sorted_ids...]} with highest wOBA first - """ - try: - data = json.loads(request.body) - player_ids = data.get("player_ids", []) - - if not player_ids: - return JsonResponse({"error": "player_ids is required"}, status=400) - - # Fetch players with the given IDs and sort by xwoba descending - players = Player.objects.filter(id__in=player_ids).order_by("-xwoba") - - # Return the sorted player IDs - sorted_ids = [player.id for player in players] - - return JsonResponse({"player_ids": sorted_ids}) - - except json.JSONDecodeError: - return JsonResponse({"error": "Invalid JSON"}, status=400) - except Exception as e: - return JsonResponse({"error": str(e)}, status=500) - - -@csrf_exempt -@require_http_methods(["GET"]) -def check_sample_players_status(request): - """Check if sample players are loaded (public endpoint). - - Returns: - - already_loaded: True if players already exist - - players_count: Number of players in database - - team_id: The team ID that players belong to - """ - players_count = Player.objects.count() - default_team, _ = Team.objects.get_or_create(pk=1) - - return JsonResponse( - { - "already_loaded": players_count > 0, - "players_count": players_count, - "team_id": default_team.id, - } - ) - - -@api_view(["POST"]) -@permission_classes([IsAdminUser]) -def load_sample_players(request): - """Load sample players from CSV (admin only). - - This endpoint requires admin authentication. - Regular users should not be able to load data into the database. - - Returns: - - success: bool - - already_loaded: bool - - players_count: Number of players in database - - team_id: The team ID that players belong to - - loaded/updated counts - """ - result = load_sample_data(team_id=1) - if not result.get("success"): - return JsonResponse(result, status=500) + @action(detail=False, methods=["get"]) + def ranked(self, request): + """Get all players ranked by wos_score.""" + from .services.player_ranking import get_ranked_players + ranked_players = get_ranked_players() + return Response({"players": ranked_players}) - return JsonResponse(result) diff --git a/backend/simulator/services/simulation.py b/backend/simulator/services/simulation.py index 067dd205..1246a00a 100644 --- a/backend/simulator/services/simulation.py +++ b/backend/simulator/services/simulation.py @@ -17,6 +17,7 @@ from typing import List from .dto import BatterStats, SimulationResult +from .player_service import PlayerService lib_path = os.path.join( os.path.dirname(__file__), "..", "..", "lib", "baseball-simulator" @@ -78,3 +79,40 @@ def simulate_lineup( std_dev=std_dev, all_scores=scores, ) + + def run_simulation_flow( + self, player_input: list | int, num_games: int, fetch_method: str + ) -> SimulationResult: + """ + Orchestrate the simulation flow: fetch players -> validate -> simulate. + + Args: + player_input: List of IDs, names, or a team ID + num_games: Number of games to simulate + fetch_method: 'ids', 'names', or 'team' + + Returns: + SimulationResult object + + Raises: + ValueError: If validation fails (wrong number of players, not found, etc.) + """ + player_service = PlayerService() + + # 1. Fetch players based on method + if fetch_method == "ids": + batter_stats = player_service.get_players_by_ids(player_input) + elif fetch_method == "names": + batter_stats = player_service.get_players_by_names(player_input) + elif fetch_method == "team": + batter_stats = player_service.get_team_players(player_input, limit=9) + if len(batter_stats) < 9: + raise ValueError( + f"Team {player_input} only has {len(batter_stats)} players with valid stats. Need exactly 9." + ) + else: + raise ValueError(f"Invalid fetch method: {fetch_method}") + + # 2. Run simulation (validation happens inside simulate_lineup) + return self.simulate_lineup(batter_stats, num_games=num_games) + diff --git a/backend/simulator/tests.py b/backend/simulator/tests.py index 67d5e73b..51bbefb1 100644 --- a/backend/simulator/tests.py +++ b/backend/simulator/tests.py @@ -22,8 +22,7 @@ from .services.simulation import SimulationService # Add baseball-simulator library to path for game simulator tests -lib_path = Path(__file__).resolve().parent.parent / \ - "lib" / "baseball-simulator" +lib_path = Path(__file__).resolve().parent.parent / "lib" / "baseball-simulator" if str(lib_path) not in sys.path: sys.path.insert(0, str(lib_path)) @@ -135,27 +134,75 @@ def test_simulate_lineup_wrong_number_of_players(self): self.assertIn("exactly 9 batters", str(context.exception)) def test_simulate_lineup_deterministic_stats(self): - """Test that more games produces more stable statistics.""" + """Test that more games produces more stable statistics (lower standard error).""" # Run with few games - result_small = self.service.simulate_lineup(self.lineup, num_games=50) + result_small = self.service.simulate_lineup(self.lineup, num_games=10) # Run with many games - result_large = self.service.simulate_lineup( - self.lineup, num_games=1000) + result_large = self.service.simulate_lineup(self.lineup, num_games=1000) - # Larger sample should have more stable (lower) std dev relative to mean - cv_small = result_small.std_dev / \ - result_small.avg_score if result_small.avg_score > 0 else 0 - cv_large = result_large.std_dev / \ - result_large.avg_score if result_large.avg_score > 0 else 0 + # Calculate Standard Error of the Mean (SEM) + # SEM = std_dev / sqrt(n) + # This represents the uncertainty in the average score and should decrease with N + sem_small = result_small.std_dev / np.sqrt(result_small.num_games) + sem_large = result_large.std_dev / np.sqrt(result_large.num_games) - # Verify both simulations produced valid results - self.assertGreater(result_small.avg_score, 0) - self.assertGreater(result_large.avg_score, 0) + # Larger sample should have lower standard error (more precise mean) + self.assertGreater(sem_small, sem_large) - # Large sample should have lower coefficient of variation - # Using a lenient check since this is probabilistic - self.assertLess(cv_large, cv_small * 2.0) + +class SimulationFlowTestCase(TestCase): + """Test SimulationService.run_simulation_flow orchestration.""" + + def setUp(self): + self.service = SimulationService() + self.team = Team.objects.create(id=1) + self.players = [] + for i in range(9): + p = Player.objects.create( + name=f"P{i}", + team=self.team, + pa=500, + hit=100, + double=20, + triple=2, + home_run=10, + strikeout=100, + walk=50 + ) + self.players.append(p) + + def test_flow_by_ids(self): + """Test flow with IDs.""" + ids = [p.id for p in self.players] + result = self.service.run_simulation_flow(ids, num_games=10, fetch_method="ids") + self.assertIsInstance(result, SimulationResult) + self.assertEqual(len(result.lineup_names), 9) + + def test_flow_by_names(self): + """Test flow with names.""" + names = [p.name for p in self.players] + result = self.service.run_simulation_flow(names, num_games=10, fetch_method="names") + self.assertIsInstance(result, SimulationResult) + + def test_flow_by_team(self): + """Test flow with team ID.""" + result = self.service.run_simulation_flow(self.team.id, num_games=10, fetch_method="team") + self.assertIsInstance(result, SimulationResult) + + def test_flow_invalid_method(self): + """Test invalid fetch method.""" + with self.assertRaises(ValueError): + self.service.run_simulation_flow([], 10, "invalid") + + def test_flow_team_not_enough_players(self): + """Test team with insufficient players.""" + empty_team = Team.objects.create(id=2) + # Add one player so we don't get "No players found" error + Player.objects.create(name="Lonely Player", team=empty_team, pa=100) + with self.assertRaises(ValueError) as ctx: + self.service.run_simulation_flow(empty_team.id, 10, "team") + self.assertIn("Need exactly 9", str(ctx.exception)) class PlayerServiceTestCase(TestCase): @@ -195,8 +242,7 @@ def test_get_players_by_ids_success(self): def test_get_players_by_ids_maintains_order(self): """Test that player order is preserved.""" - player_ids = [self.players[5].id, - self.players[2].id, self.players[8].id] + player_ids = [self.players[5].id, self.players[2].id, self.players[8].id] batter_stats = self.service.get_players_by_ids(player_ids) self.assertEqual(batter_stats[0].name, "Test Player 6") @@ -284,8 +330,7 @@ def test_simulate_by_ids_success(self): def test_simulate_by_ids_invalid_count(self): """Test that endpoint rejects wrong number of players.""" url = "/api/v1/simulator/simulate-by-ids/" - data = {"player_ids": [self.players[0].id], - "num_games": 100} # Only 1 player + data = {"player_ids": [self.players[0].id], "num_games": 100} # Only 1 player response = self.client.post(url, data, format="json") @@ -315,13 +360,45 @@ def test_simulate_by_team_success(self): def test_simulate_by_names_success(self): """Test simulation by player names endpoint.""" url = "/api/v1/simulator/simulate-by-names/" - data = {"player_names": [ - p.name for p in self.players], "num_games": 100} + data = {"player_names": [p.name for p in self.players], "num_games": 100} response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, status.HTTP_200_OK) + def test_simulate_by_ids_player_not_found(self): + """Test simulation with non-existent player ID.""" + url = "/api/v1/simulator/simulate-by-ids/" + invalid_ids = [99999] * 9 # 9 non-existent IDs + data = {"player_ids": invalid_ids, "num_games": 100} + + response = self.client.post(url, data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("error", response.data) + + def test_simulate_by_names_player_not_found(self): + """Test simulation with non-existent player name.""" + url = "/api/v1/simulator/simulate-by-names/" + invalid_names = ["NonExistent Player"] * 9 + data = {"player_names": invalid_names, "num_games": 100} + + response = self.client.post(url, data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("error", response.data) + + def test_simulate_by_team_no_players(self): + """Test simulation with team that has no players.""" + empty_team = Team.objects.create(id=999) + url = "/api/v1/simulator/simulate-by-team/" + data = {"team_id": empty_team.id, "num_games": 100} + + response = self.client.post(url, data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("error", response.data) + class ParallelGameIntegrationTests(TestCase): """Integration tests for ParallelGame comparing statistics.""" @@ -396,13 +473,11 @@ def test_parallel_speedup(self): # Run with 1 process vs multiple processes # We can't easily time this in tests, but we can verify both work - game_single = self.ParallelGame( - lineup=lineup, num_games=100, num_processes=1) + game_single = self.ParallelGame(lineup=lineup, num_games=100, num_processes=1) game_single.play() scores_single = game_single.get_scores() - game_multi = self.ParallelGame( - lineup=lineup, num_games=100, num_processes=4) + game_multi = self.ParallelGame(lineup=lineup, num_games=100, num_processes=4) game_multi.play() scores_multi = game_multi.get_scores() @@ -419,3 +494,131 @@ def test_parallel_speedup(self): self.assertGreater(avg_multi, 6.0) self.assertLess(avg_single, 16.0) self.assertLess(avg_multi, 16.0) + + +from unittest.mock import MagicMock, patch +from simulator.views import _handle_simulation_request + +class SimulatorViewsCoverageTest(TestCase): + """Targeted tests for simulator/views.py coverage.""" + + def setUp(self): + self.client = APIClient() + + @patch("simulator.views.SimulationService") + def test_handle_simulation_request_empty_results(self, mock_service_cls): + """Test _handle_simulation_request when simulation returns no scores.""" + # Setup mock service + mock_service = MagicMock() + mock_service_cls.return_value = mock_service + + # Setup mock result with empty scores + mock_result = MagicMock() + mock_result.all_scores = [] # Empty list triggers the error + mock_service.run_simulation_flow.return_value = mock_result + + # Call the helper directly + response = _handle_simulation_request([1, 2, 3], 100, "ids") + + self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) + self.assertIn("Simulation produced no results", response.data["error"]) + + @patch("simulator.views.SimulationService") + def test_handle_simulation_request_unexpected_error(self, mock_service_cls): + """Test _handle_simulation_request when an unexpected exception occurs.""" + # Setup mock to raise generic exception + mock_service = MagicMock() + mock_service_cls.return_value = mock_service + mock_service.run_simulation_flow.side_effect = Exception("Unexpected boom") + + # Call the helper directly + response = _handle_simulation_request([1, 2, 3], 100, "ids") + + self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) + self.assertIn("An unexpected error occurred", response.data["error"]) + self.assertIn("Unexpected boom", response.data["detail"]) + + def test_simulate_by_player_names_invalid_serializer(self): + """Test simulate_by_player_names with invalid data.""" + # Missing player_names + data = {"num_games": 100} + + # We need a user to authenticate + user = User.objects.create_user(username="testuser_cov", password="password") + self.client.force_authenticate(user=user) + + url = "/api/v1/simulator/simulate-by-names/" + response = self.client.post(url, data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("player_names", response.data) + + def test_simulate_by_team_invalid_serializer(self): + """Test simulate_by_team with invalid data.""" + # Missing team_id + data = {"num_games": 100} + + user = User.objects.create_user(username="testuser2_cov", password="password") + self.client.force_authenticate(user=user) + + url = "/api/v1/simulator/simulate-by-team/" + response = self.client.post(url, data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("team_id", response.data) + + +class SimulatorServicesCoverageTest(TestCase): + """Targeted tests for simulator services coverage.""" + + def test_batter_stats_invalid_probabilities(self): + """Test that BatterStats raises ValueError when probabilities sum > 1.0.""" + # Create stats that will result in probabilities > 1.0 + stats = BatterStats( + name="Impossible Player", + plate_appearances=10, + hits=5, + doubles=0, + triples=0, + home_runs=0, + strikeouts=6, # 5 + 6 = 11 > 10 + walks=0 + ) + + with self.assertRaises(ValueError) as cm: + stats.to_probabilities() + + self.assertIn("Invalid data for player", str(cm.exception)) + self.assertIn("probabilities sum to", str(cm.exception)) + + def test_convert_to_batter_stats_zero_pa(self): + """Test _convert_to_batter_stats with 0 plate appearances.""" + service = PlayerService() + + # Mock a player object + mock_player = MagicMock() + mock_player.name = "Bench Warmer" + mock_player.pa = 0 + + with self.assertRaises(ValueError) as cm: + service._convert_to_batter_stats(mock_player) + + self.assertIn("has no plate appearances", str(cm.exception)) + + def test_batter_stats_zero_pa_default(self): + """Test BatterStats.to_probabilities with 0 PA returns default outs.""" + stats = BatterStats( + name="No PA Player", + plate_appearances=0, + hits=0, + doubles=0, + triples=0, + home_runs=0, + strikeouts=0, + walks=0 + ) + + probs = stats.to_probabilities() + # Should be [K, out, walk, 1B, 2B, 3B, HR] + # Default is [0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0] + self.assertEqual(probs, [0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0]) diff --git a/backend/simulator/views.py b/backend/simulator/views.py index 6df37307..c601e4b2 100644 --- a/backend/simulator/views.py +++ b/backend/simulator/views.py @@ -18,13 +18,7 @@ from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response -from .serializers import ( - PlayerInputSerializer, - PlayerNameInputSerializer, - SimulationResultSerializer, - TeamInputSerializer, -) -from .services.player_service import PlayerService +from .serializers import PlayerInputSerializer, PlayerNameInputSerializer, SimulationResultSerializer, TeamInputSerializer from .services.simulation import SimulationService logger = logging.getLogger(__name__) @@ -33,90 +27,49 @@ def _handle_simulation_request(player_input, num_games, fetch_method): """ Helper to handle simulation request with consistent error handling. - - Args: - player_input: Player IDs, names, or team ID - num_games: Number of games to simulate - fetch_method: 'ids', 'names', or 'team' - - Returns: - Response object with simulation results or error + Delegates orchestration to SimulationService. """ try: - player_service = PlayerService() - - # Fetch players based on method - if fetch_method == "ids": - batter_stats = player_service.get_players_by_ids(player_input) - elif fetch_method == "names": - batter_stats = player_service.get_players_by_names(player_input) - elif fetch_method == "team": - batter_stats = player_service.get_team_players( - player_input, limit=9) - if len(batter_stats) < 9: - return Response( - { - "error": ( - f"Team {player_input} only has " - f"{len(batter_stats)} players. Need 9." - ) - }, - status=status.HTTP_400_BAD_REQUEST, - ) - else: - return Response({"error": "Invalid fetch method"}, - status=status.HTTP_500_INTERNAL_SERVER_ERROR) - - return _run_simulation_and_format_response(batter_stats, num_games) + service = SimulationService() + result = service.run_simulation_flow(player_input, num_games, fetch_method) + + # Handle empty scores edge case + if not result.all_scores: + return Response( + {"error": "Simulation produced no results. Please check input data."}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) + + response_data = { + "lineup": result.lineup_names, + "num_games": result.num_games, + "avg_score": result.avg_score, + "median_score": result.median_score, + "std_dev": result.std_dev, + "min_score": min(result.all_scores), + "max_score": max(result.all_scores), + "score_distribution": _calculate_distribution(result.all_scores), + } + + output_serializer = SimulationResultSerializer(response_data) + return Response(output_serializer.data, status=status.HTTP_200_OK) except ValueError as e: # Player not found or data validation error logger.warning(f"ValueError in simulation: {str(e)}") return Response( - {"error": str( - e), "hint": "Check that all player IDs/names exist and have valid statistics."}, - status=status.HTTP_404_NOT_FOUND, + {"error": str(e), "hint": "Check that all player IDs/names exist and have valid statistics."}, + status=status.HTTP_400_BAD_REQUEST, ) except Exception as e: # Unexpected error - log for debugging logger.error(f"Simulation failed: {str(e)}", exc_info=True) return Response( - {"error": "An unexpected error occurred during simulation.", - "detail": str(e)}, + {"error": "An unexpected error occurred during simulation.", "detail": str(e)}, status=status.HTTP_500_INTERNAL_SERVER_ERROR, ) -def _run_simulation_and_format_response(batter_stats, num_games): - """ - Helper function to run simulation and format response. - Eliminates code duplication across view functions. - """ - service = SimulationService() - result = service.simulate_lineup(batter_stats, num_games=num_games) - - # Handle empty scores edge case - if not result.all_scores: - return Response( - {"error": "Simulation produced no results. Check input data."}, - status=status.HTTP_500_INTERNAL_SERVER_ERROR - ) - - response_data = { - "lineup": result.lineup_names, - "num_games": result.num_games, - "avg_score": result.avg_score, - "median_score": result.median_score, - "std_dev": result.std_dev, - "min_score": min(result.all_scores), - "max_score": max(result.all_scores), - "score_distribution": _calculate_distribution(result.all_scores), - } - - output_serializer = SimulationResultSerializer(response_data) - return Response(output_serializer.data, status=status.HTTP_200_OK) - - @api_view(["POST"]) @permission_classes([IsAuthenticated]) def simulate_by_player_ids(request):