cal/major-domo-v2
1
0
Fork 0
Code Issues 6 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix: roster validation now includes org affiliate transactions (closes #49)

Browse Source 
load_existing_transactions only queried for the base team abbreviation
(e.g. "POR"), missing trades involving MiL/IL affiliates ("PORMIL",
"PORIL"). This caused false "too many players" errors when a pending
trade would have cleared a roster spot.

- get_team_transactions now accepts Union[str, List[str]] for team_abbrev
- load_existing_transactions queries all org affiliates [BASE, BASEMiL, BASEIL]
- Added 5 tests covering the fix and backwards compatibility

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Cal Corum 2026-03-01 10:45:21 -06:00
parent a80addc742
commit aa27769ed6
3 changed files with 627 additions and 229 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

296
services/transaction_builder.py
View File

@ -3,6 +3,7 @@ Transaction Builder Service
Handles the complex logic for building multi-move transactions interactively. Handles the complex logic for building multi-move transactions interactively.
""" """
import logging import logging
from typing import Dict, List, Optional from typing import Dict, List, Optional
from dataclasses import dataclass from dataclasses import dataclass
@ -18,7 +19,7 @@ from services.transaction_service import transaction_service
from services.league_service import league_service from services.league_service import league_service
from models.team import RosterType from models.team import RosterType
logger = logging.getLogger(f'{__name__}.TransactionBuilder') logger = logging.getLogger(f"{__name__}.TransactionBuilder")
# Removed MoveAction enum - using simple from/to roster locations instead # Removed MoveAction enum - using simple from/to roster locations instead
@ -27,53 +28,87 @@ logger = logging.getLogger(f'{__name__}.TransactionBuilder')
@dataclass @dataclass
class TransactionMove: class TransactionMove:
"""Individual move within a transaction.""" """Individual move within a transaction."""
player: Player player: Player
from_roster: RosterType from_roster: RosterType
to_roster: RosterType to_roster: RosterType
from_team: Optional[Team] = None from_team: Optional[Team] = None
to_team: Optional[Team] = None to_team: Optional[Team] = None
@property @property
def description(self) -> str: def description(self) -> str:
"""Human readable move description.""" """Human readable move description."""
# Determine emoji and format based on from/to locations # Determine emoji and format based on from/to locations
if self.from_roster == RosterType.FREE_AGENCY and self.to_roster != RosterType.FREE_AGENCY: if (
self.from_roster == RosterType.FREE_AGENCY
and self.to_roster != RosterType.FREE_AGENCY
):
# Add from Free Agency # Add from Free Agency
emoji = "➕" emoji = "➕"
return f"{emoji} {self.player.name}: FA → {self.to_team.abbrev} ({self.to_roster.value.upper()})" return f"{emoji} {self.player.name}: FA → {self.to_team.abbrev} ({self.to_roster.value.upper()})"
elif self.from_roster != RosterType.FREE_AGENCY and self.to_roster == RosterType.FREE_AGENCY: elif (
self.from_roster != RosterType.FREE_AGENCY
and self.to_roster == RosterType.FREE_AGENCY
):
# Drop to Free Agency # Drop to Free Agency
emoji = "➖" emoji = "➖"
return f"{emoji} {self.player.name}: {self.from_team.abbrev} ({self.from_roster.value.upper()}) → FA" return f"{emoji} {self.player.name}: {self.from_team.abbrev} ({self.from_roster.value.upper()}) → FA"
elif self.from_roster == RosterType.MINOR_LEAGUE and self.to_roster == RosterType.MAJOR_LEAGUE: elif (
self.from_roster == RosterType.MINOR_LEAGUE
and self.to_roster == RosterType.MAJOR_LEAGUE
):
# Recall from MiL to ML # Recall from MiL to ML
emoji = "⬆️" emoji = "⬆️"
return f"{emoji} {self.player.name}: {self.from_team.abbrev} (MiL) → {self.to_team.abbrev} (ML)" return f"{emoji} {self.player.name}: {self.from_team.abbrev} (MiL) → {self.to_team.abbrev} (ML)"
elif self.from_roster == RosterType.MAJOR_LEAGUE and self.to_roster == RosterType.MINOR_LEAGUE: elif (
self.from_roster == RosterType.MAJOR_LEAGUE
and self.to_roster == RosterType.MINOR_LEAGUE
):
# Demote from ML to MiL # Demote from ML to MiL
emoji = "⬇️" emoji = "⬇️"
return f"{emoji} {self.player.name}: {self.from_team.abbrev} (ML) → {self.to_team.abbrev} (MiL)" return f"{emoji} {self.player.name}: {self.from_team.abbrev} (ML) → {self.to_team.abbrev} (MiL)"
elif self.to_roster == RosterType.INJURED_LIST: elif self.to_roster == RosterType.INJURED_LIST:
# Move to Injured List # Move to Injured List
emoji = "🏥" emoji = "🏥"
from_desc = "FA" if self.from_roster == RosterType.FREE_AGENCY else f"{self.from_team.abbrev} ({self.from_roster.value.upper()})" from_desc = (
return f"{emoji} {self.player.name}: {from_desc} → {self.to_team.abbrev} (IL)" "FA"
if self.from_roster == RosterType.FREE_AGENCY
else f"{self.from_team.abbrev} ({self.from_roster.value.upper()})"
)
return (
f"{emoji} {self.player.name}: {from_desc} → {self.to_team.abbrev} (IL)"
)
elif self.from_roster == RosterType.INJURED_LIST: elif self.from_roster == RosterType.INJURED_LIST:
# Return from Injured List # Return from Injured List
emoji = "💊" emoji = "💊"
to_desc = "FA" if self.to_roster == RosterType.FREE_AGENCY else f"{self.to_team.abbrev} ({self.to_roster.value.upper()})" to_desc = (
return f"{emoji} {self.player.name}: {self.from_team.abbrev} (IL) → {to_desc}" "FA"
if self.to_roster == RosterType.FREE_AGENCY
else f"{self.to_team.abbrev} ({self.to_roster.value.upper()})"
)
return (
f"{emoji} {self.player.name}: {self.from_team.abbrev} (IL) → {to_desc}"
)
else: else:
# Generic move # Generic move
emoji = "🔄" emoji = "🔄"
from_desc = "FA" if self.from_roster == RosterType.FREE_AGENCY else f"{self.from_team.abbrev} ({self.from_roster.value.upper()})" from_desc = (
to_desc = "FA" if self.to_roster == RosterType.FREE_AGENCY else f"{self.to_team.abbrev} ({self.to_roster.value.upper()})" "FA"
if self.from_roster == RosterType.FREE_AGENCY
else f"{self.from_team.abbrev} ({self.from_roster.value.upper()})"
)
to_desc = (
"FA"
if self.to_roster == RosterType.FREE_AGENCY
else f"{self.to_team.abbrev} ({self.to_roster.value.upper()})"
)
return f"{emoji} {self.player.name}: {from_desc} → {to_desc}" return f"{emoji} {self.player.name}: {from_desc} → {to_desc}"
@dataclass @dataclass
class RosterValidationResult: class RosterValidationResult:
"""Results of roster validation.""" """Results of roster validation."""
is_legal: bool is_legal: bool
major_league_count: int major_league_count: int
minor_league_count: int minor_league_count: int
@ -88,7 +123,7 @@ class RosterValidationResult:
pre_existing_ml_swar_change: float = 0.0 pre_existing_ml_swar_change: float = 0.0
pre_existing_mil_swar_change: float = 0.0 pre_existing_mil_swar_change: float = 0.0
pre_existing_transaction_count: int = 0 pre_existing_transaction_count: int = 0
@property @property
def major_league_status(self) -> str: def major_league_status(self) -> str:
"""Status string for major league roster.""" """Status string for major league roster."""
@ -96,7 +131,7 @@ class RosterValidationResult:
return f"❌ Major League: {self.major_league_count}/{self.major_league_limit} (Over limit!)" return f"❌ Major League: {self.major_league_count}/{self.major_league_limit} (Over limit!)"
else: else:
return f"✅ Major League: {self.major_league_count}/{self.major_league_limit} (Legal)" return f"✅ Major League: {self.major_league_count}/{self.major_league_limit} (Legal)"
@property @property
def minor_league_status(self) -> str: def minor_league_status(self) -> str:
"""Status string for minor league roster.""" """Status string for minor league roster."""
@ -124,7 +159,9 @@ class RosterValidationResult:
if self.pre_existing_transaction_count == 0: if self.pre_existing_transaction_count == 0:
return "" return ""
total_swar_change = self.pre_existing_ml_swar_change + self.pre_existing_mil_swar_change total_swar_change = (
self.pre_existing_ml_swar_change + self.pre_existing_mil_swar_change
)
if total_swar_change == 0: if total_swar_change == 0:
return f"ℹ️ **Pre-existing Moves**: {self.pre_existing_transaction_count} scheduled moves (no sWAR impact)" return f"ℹ️ **Pre-existing Moves**: {self.pre_existing_transaction_count} scheduled moves (no sWAR impact)"
@ -136,11 +173,11 @@ class RosterValidationResult:
class TransactionBuilder: class TransactionBuilder:
"""Interactive transaction builder for complex multi-move transactions.""" """Interactive transaction builder for complex multi-move transactions."""
def __init__(self, team: Team, user_id: int, season: int = get_config().sba_season): def __init__(self, team: Team, user_id: int, season: int = get_config().sba_season):
""" """
Initialize transaction builder. Initialize transaction builder.
Args: Args:
team: Team making the transaction team: Team making the transaction
user_id: Discord user ID of the GM user_id: Discord user ID of the GM
@ -151,7 +188,7 @@ class TransactionBuilder:
self.season = season self.season = season
self.moves: List[TransactionMove] = [] self.moves: List[TransactionMove] = []
self.created_at = datetime.now(timezone.utc) self.created_at = datetime.now(timezone.utc)
# Cache for roster data # Cache for roster data
self._current_roster: Optional[TeamRoster] = None self._current_roster: Optional[TeamRoster] = None
self._roster_loaded = False self._roster_loaded = False
@ -160,8 +197,10 @@ class TransactionBuilder:
self._existing_transactions: Optional[List[Transaction]] = None self._existing_transactions: Optional[List[Transaction]] = None
self._existing_transactions_loaded = False self._existing_transactions_loaded = False
logger.info(f"TransactionBuilder initialized for {team.abbrev} by user {user_id}") logger.info(
f"TransactionBuilder initialized for {team.abbrev} by user {user_id}"
)
async def load_roster_data(self) -> None: async def load_roster_data(self) -> None:
"""Load current roster data for the team.""" """Load current roster data for the team."""
if self._roster_loaded: if self._roster_loaded:
@ -177,29 +216,42 @@ class TransactionBuilder:
self._roster_loaded = True self._roster_loaded = True
async def load_existing_transactions(self, next_week: int) -> None: async def load_existing_transactions(self, next_week: int) -> None:
"""Load pre-existing transactions for next week.""" """
Load pre-existing transactions for next week.
Queries for all organizational affiliates (ML, MiL, IL) to ensure
trades involving affiliate teams are included in roster projections.
"""
if self._existing_transactions_loaded: if self._existing_transactions_loaded:
return return
try: try:
self._existing_transactions = await transaction_service.get_team_transactions( # Include all org affiliates so trades involving MiL/IL teams are captured
team_abbrev=self.team.abbrev, base_abbrev = self.team._get_base_abbrev()
season=self.season, org_abbrevs = [base_abbrev, f"{base_abbrev}MIL", f"{base_abbrev}IL"]
cancelled=False,
week_start=next_week self._existing_transactions = (
await transaction_service.get_team_transactions(
team_abbrev=org_abbrevs,
season=self.season,
cancelled=False,
week_start=next_week,
)
) )
self._existing_transactions_loaded = True self._existing_transactions_loaded = True
logger.debug(f"Loaded {len(self._existing_transactions or [])} existing transactions for {self.team.abbrev} week {next_week}") logger.debug(
f"Loaded {len(self._existing_transactions or [])} existing transactions for {self.team.abbrev} org ({org_abbrevs}) week {next_week}"
)
except Exception as e: except Exception as e:
logger.error(f"Failed to load existing transactions: {e}") logger.error(f"Failed to load existing transactions: {e}")
self._existing_transactions = [] self._existing_transactions = []
self._existing_transactions_loaded = True self._existing_transactions_loaded = True
async def add_move( async def add_move(
self, self,
move: TransactionMove, move: TransactionMove,
next_week: Optional[int] = None, next_week: Optional[int] = None,
check_pending_transactions: bool = True check_pending_transactions: bool = True,
) -> tuple[bool, str]: ) -> tuple[bool, str]:
""" """
Add a move to the transaction. Add a move to the transaction.
@ -216,14 +268,20 @@ class TransactionBuilder:
# Check if player is already in a move in this transaction builder # Check if player is already in a move in this transaction builder
existing_move = self.get_move_for_player(move.player.id) existing_move = self.get_move_for_player(move.player.id)
if existing_move: if existing_move:
error_msg = f"Player {move.player.name} already has a move in this transaction" error_msg = (
f"Player {move.player.name} already has a move in this transaction"
)
logger.warning(error_msg) logger.warning(error_msg)
return False, error_msg return False, error_msg
# Check if from_team and to_team are the same AND from_roster and to_roster are the same # Check if from_team and to_team are the same AND from_roster and to_roster are the same
# (when both teams are not None) - this would be a meaningless move # (when both teams are not None) - this would be a meaningless move
if (move.from_team is not None and move.to_team is not None and if (
move.from_team.id == move.to_team.id and move.from_roster == move.to_roster): move.from_team is not None
and move.to_team is not None
and move.from_team.id == move.to_team.id
and move.from_roster == move.to_roster
):
error_msg = f"Cannot move {move.player.name} from {move.from_team.abbrev} ({move.from_roster.value.upper()}) to {move.to_team.abbrev} ({move.to_roster.value.upper()}) - player is already in that location" error_msg = f"Cannot move {move.player.name} from {move.from_team.abbrev} ({move.from_roster.value.upper()}) to {move.to_team.abbrev} ({move.to_roster.value.upper()}) - player is already in that location"
logger.warning(error_msg) logger.warning(error_msg)
return False, error_msg return False, error_msg
@ -237,13 +295,15 @@ class TransactionBuilder:
current_state = await league_service.get_current_state() current_state = await league_service.get_current_state()
next_week = (current_state.week + 1) if current_state else 1 next_week = (current_state.week + 1) if current_state else 1
except Exception as e: except Exception as e:
logger.warning(f"Could not get current week for pending transaction check: {e}") logger.warning(
f"Could not get current week for pending transaction check: {e}"
)
next_week = 1 next_week = 1
is_pending, claiming_team = await transaction_service.is_player_in_pending_transaction( is_pending, claiming_team = (
player_id=move.player.id, await transaction_service.is_player_in_pending_transaction(
week=next_week, player_id=move.player.id, week=next_week, season=self.season
season=self.season )
) )
if is_pending: if is_pending:
@ -254,34 +314,36 @@ class TransactionBuilder:
self.moves.append(move) self.moves.append(move)
logger.info(f"Added move: {move.description}") logger.info(f"Added move: {move.description}")
return True, "" return True, ""
def remove_move(self, player_id: int) -> bool: def remove_move(self, player_id: int) -> bool:
""" """
Remove a move for a specific player. Remove a move for a specific player.
Args: Args:
player_id: ID of player to remove move for player_id: ID of player to remove move for
Returns: Returns:
True if move was removed True if move was removed
""" """
original_count = len(self.moves) original_count = len(self.moves)
self.moves = [move for move in self.moves if move.player.id != player_id] self.moves = [move for move in self.moves if move.player.id != player_id]
removed = len(self.moves) < original_count removed = len(self.moves) < original_count
if removed: if removed:
logger.info(f"Removed move for player {player_id}") logger.info(f"Removed move for player {player_id}")
return removed return removed
def get_move_for_player(self, player_id: int) -> Optional[TransactionMove]: def get_move_for_player(self, player_id: int) -> Optional[TransactionMove]:
"""Get the move for a specific player if it exists.""" """Get the move for a specific player if it exists."""
for move in self.moves: for move in self.moves:
if move.player.id == player_id: if move.player.id == player_id:
return move return move
return None return None
async def validate_transaction(self, next_week: Optional[int] = None) -> RosterValidationResult: async def validate_transaction(
self, next_week: Optional[int] = None
) -> RosterValidationResult:
""" """
Validate the current transaction and return detailed results. Validate the current transaction and return detailed results.
@ -296,17 +358,17 @@ class TransactionBuilder:
# Load existing transactions if next_week is provided # Load existing transactions if next_week is provided
if next_week is not None: if next_week is not None:
await self.load_existing_transactions(next_week) await self.load_existing_transactions(next_week)
if not self._current_roster: if not self._current_roster:
return RosterValidationResult( return RosterValidationResult(
is_legal=False, is_legal=False,
major_league_count=0, major_league_count=0,
minor_league_count=0, minor_league_count=0,
warnings=[], warnings=[],
errors=["Could not load current roster data"], errors=["Could not load current roster data"],
suggestions=[] suggestions=[],
) )
# Calculate roster changes from moves # Calculate roster changes from moves
ml_changes = 0 ml_changes = 0
mil_changes = 0 mil_changes = 0
@ -315,8 +377,12 @@ class TransactionBuilder:
suggestions = [] suggestions = []
# Calculate current sWAR for each roster # Calculate current sWAR for each roster
current_ml_swar = sum(player.wara for player in self._current_roster.active_players) current_ml_swar = sum(
current_mil_swar = sum(player.wara for player in self._current_roster.minor_league_players) player.wara for player in self._current_roster.active_players
)
current_mil_swar = sum(
player.wara for player in self._current_roster.minor_league_players
)
# Track sWAR changes from moves # Track sWAR changes from moves
ml_swar_changes = 0.0 ml_swar_changes = 0.0
@ -371,46 +437,66 @@ class TransactionBuilder:
for move in self.moves: for move in self.moves:
# Log move being processed for diagnostics # Log move being processed for diagnostics
logger.debug(f"🔍 VALIDATION: Processing move - {move.player.name} (ID={move.player.id})") logger.debug(
logger.debug(f"🔍 VALIDATION: from_roster={move.from_roster.value}, to_roster={move.to_roster.value}") f"🔍 VALIDATION: Processing move - {move.player.name} (ID={move.player.id})"
)
logger.debug(
f"🔍 VALIDATION: from_roster={move.from_roster.value}, to_roster={move.to_roster.value}"
)
# Calculate roster changes based on from/to locations # Calculate roster changes based on from/to locations
if move.from_roster == RosterType.MAJOR_LEAGUE: if move.from_roster == RosterType.MAJOR_LEAGUE:
ml_changes -= 1 ml_changes -= 1
ml_swar_changes -= move.player.wara ml_swar_changes -= move.player.wara
logger.debug(f"🔍 VALIDATION: ML decrement - ml_changes now {ml_changes}") logger.debug(
f"🔍 VALIDATION: ML decrement - ml_changes now {ml_changes}"
)
elif move.from_roster == RosterType.MINOR_LEAGUE: elif move.from_roster == RosterType.MINOR_LEAGUE:
mil_changes -= 1 mil_changes -= 1
mil_swar_changes -= move.player.wara mil_swar_changes -= move.player.wara
logger.debug(f"🔍 VALIDATION: MiL decrement - mil_changes now {mil_changes}") logger.debug(
f"🔍 VALIDATION: MiL decrement - mil_changes now {mil_changes}"
)
# Note: INJURED_LIST and FREE_AGENCY don't count toward ML roster limit # Note: INJURED_LIST and FREE_AGENCY don't count toward ML roster limit
if move.to_roster == RosterType.MAJOR_LEAGUE: if move.to_roster == RosterType.MAJOR_LEAGUE:
ml_changes += 1 ml_changes += 1
ml_swar_changes += move.player.wara ml_swar_changes += move.player.wara
logger.debug(f"🔍 VALIDATION: ML increment - ml_changes now {ml_changes}") logger.debug(
f"🔍 VALIDATION: ML increment - ml_changes now {ml_changes}"
)
elif move.to_roster == RosterType.MINOR_LEAGUE: elif move.to_roster == RosterType.MINOR_LEAGUE:
mil_changes += 1 mil_changes += 1
mil_swar_changes += move.player.wara mil_swar_changes += move.player.wara
logger.debug(f"🔍 VALIDATION: MiL increment - mil_changes now {mil_changes}") logger.debug(
f"🔍 VALIDATION: MiL increment - mil_changes now {mil_changes}"
)
# Note: INJURED_LIST and FREE_AGENCY don't count toward ML roster limit # Note: INJURED_LIST and FREE_AGENCY don't count toward ML roster limit
# Calculate projected roster sizes and sWAR # Calculate projected roster sizes and sWAR
# Only Major League players count toward ML roster limit (IL and MiL are separate) # Only Major League players count toward ML roster limit (IL and MiL are separate)
current_ml_size = len(self._current_roster.active_players) current_ml_size = len(self._current_roster.active_players)
current_mil_size = len(self._current_roster.minor_league_players) current_mil_size = len(self._current_roster.minor_league_players)
logger.debug(f"🔍 VALIDATION: Current roster - ML:{current_ml_size}, MiL:{current_mil_size}") logger.debug(
logger.debug(f"🔍 VALIDATION: Changes calculated - ml_changes:{ml_changes}, mil_changes:{mil_changes}") f"🔍 VALIDATION: Current roster - ML:{current_ml_size}, MiL:{current_mil_size}"
)
logger.debug(
f"🔍 VALIDATION: Changes calculated - ml_changes:{ml_changes}, mil_changes:{mil_changes}"
)
projected_ml_size = current_ml_size + ml_changes projected_ml_size = current_ml_size + ml_changes
projected_mil_size = current_mil_size + mil_changes projected_mil_size = current_mil_size + mil_changes
projected_ml_swar = current_ml_swar + ml_swar_changes projected_ml_swar = current_ml_swar + ml_swar_changes
projected_mil_swar = current_mil_swar + mil_swar_changes projected_mil_swar = current_mil_swar + mil_swar_changes
logger.debug(f"🔍 VALIDATION: Projected roster - ML:{projected_ml_size}, MiL:{projected_mil_size}") logger.debug(
logger.debug(f"🔍 VALIDATION: Projected sWAR - ML:{projected_ml_swar:.2f}, MiL:{projected_mil_swar:.2f}") f"🔍 VALIDATION: Projected roster - ML:{projected_ml_size}, MiL:{projected_mil_size}"
)
logger.debug(
f"🔍 VALIDATION: Projected sWAR - ML:{projected_ml_swar:.2f}, MiL:{projected_mil_swar:.2f}"
)
# Get current week and config to determine roster limits # Get current week and config to determine roster limits
config = get_config() config = get_config()
try: try:
@ -432,38 +518,54 @@ class TransactionBuilder:
else: else:
ml_limit = config.ml_roster_limit_late ml_limit = config.ml_roster_limit_late
mil_limit = config.mil_roster_limit_late mil_limit = config.mil_roster_limit_late
# Validate roster limits # Validate roster limits
is_legal = True is_legal = True
if projected_ml_size > ml_limit: if projected_ml_size > ml_limit:
is_legal = False is_legal = False
errors.append(f"Major League roster would have {projected_ml_size} players (limit: {ml_limit})") errors.append(
suggestions.append(f"Drop {projected_ml_size - ml_limit} ML player(s) to make roster legal") f"Major League roster would have {projected_ml_size} players (limit: {ml_limit})"
)
suggestions.append(
f"Drop {projected_ml_size - ml_limit} ML player(s) to make roster legal"
)
elif projected_ml_size < 0: elif projected_ml_size < 0:
is_legal = False is_legal = False
errors.append("Cannot have negative players on Major League roster") errors.append("Cannot have negative players on Major League roster")
if projected_mil_size > mil_limit: if projected_mil_size > mil_limit:
is_legal = False is_legal = False
errors.append(f"Minor League roster would have {projected_mil_size} players (limit: {mil_limit})") errors.append(
suggestions.append(f"Drop {projected_mil_size - mil_limit} MiL player(s) to make roster legal") f"Minor League roster would have {projected_mil_size} players (limit: {mil_limit})"
)
suggestions.append(
f"Drop {projected_mil_size - mil_limit} MiL player(s) to make roster legal"
)
elif projected_mil_size < 0: elif projected_mil_size < 0:
is_legal = False is_legal = False
errors.append("Cannot have negative players on Minor League roster") errors.append("Cannot have negative players on Minor League roster")
# Validate sWAR cap # Validate sWAR cap
# Use team-specific cap if set, otherwise fall back to config default # Use team-specific cap if set, otherwise fall back to config default
team_swar_cap = self.team.salary_cap if self.team.salary_cap is not None else config.swar_cap_limit team_swar_cap = (
self.team.salary_cap
if self.team.salary_cap is not None
else config.swar_cap_limit
)
if projected_ml_swar > team_swar_cap: if projected_ml_swar > team_swar_cap:
is_legal = False is_legal = False
errors.append(f"Major League sWAR would be {projected_ml_swar:.2f} (cap: {team_swar_cap:.1f})") errors.append(
f"Major League sWAR would be {projected_ml_swar:.2f} (cap: {team_swar_cap:.1f})"
)
over_cap = projected_ml_swar - team_swar_cap over_cap = projected_ml_swar - team_swar_cap
suggestions.append(f"Remove {over_cap:.2f} sWAR from ML roster to be under cap") suggestions.append(
f"Remove {over_cap:.2f} sWAR from ML roster to be under cap"
)
# Add suggestions for empty transaction # Add suggestions for empty transaction
if not self.moves: if not self.moves:
suggestions.append("Add player moves to build your transaction") suggestions.append("Add player moves to build your transaction")
return RosterValidationResult( return RosterValidationResult(
is_legal=is_legal, is_legal=is_legal,
major_league_count=projected_ml_size, major_league_count=projected_ml_size,
@ -478,10 +580,12 @@ class TransactionBuilder:
major_league_swar_cap=team_swar_cap, major_league_swar_cap=team_swar_cap,
pre_existing_ml_swar_change=pre_existing_ml_swar_change, pre_existing_ml_swar_change=pre_existing_ml_swar_change,
pre_existing_mil_swar_change=pre_existing_mil_swar_change, pre_existing_mil_swar_change=pre_existing_mil_swar_change,
pre_existing_transaction_count=pre_existing_count pre_existing_transaction_count=pre_existing_count,
) )
async def submit_transaction(self, week: int, check_existing_transactions: bool = True) -> List[Transaction]: async def submit_transaction(
self, week: int, check_existing_transactions: bool = True
) -> List[Transaction]:
""" """
Submit the transaction by creating individual Transaction models. Submit the transaction by creating individual Transaction models.
@ -504,11 +608,13 @@ class TransactionBuilder:
validation = await self.validate_transaction() validation = await self.validate_transaction()
if not validation.is_legal: if not validation.is_legal:
raise ValueError(f"Cannot submit illegal transaction: {', '.join(validation.errors)}") raise ValueError(
f"Cannot submit illegal transaction: {', '.join(validation.errors)}"
)
transactions = [] transactions = []
move_id = f"Season-{self.season:03d}-Week-{week:02d}-{int(self.created_at.timestamp())}" move_id = f"Season-{self.season:03d}-Week-{week:02d}-{int(self.created_at.timestamp())}"
# Create FA team for drops using config value # Create FA team for drops using config value
config = get_config() config = get_config()
fa_team = Team( fa_team = Team(
@ -516,9 +622,9 @@ class TransactionBuilder:
abbrev="FA", abbrev="FA",
sname="Free Agents", sname="Free Agents",
lname="Free Agency", lname="Free Agency",
season=self.season season=self.season,
) # type: ignore ) # type: ignore
for move in self.moves: for move in self.moves:
# Determine old and new teams based on roster locations # Determine old and new teams based on roster locations
# We need to map RosterType to the actual team (ML, MiL, or IL affiliate) # We need to map RosterType to the actual team (ML, MiL, or IL affiliate)
@ -553,7 +659,7 @@ class TransactionBuilder:
# For cases where we don't have specific teams, fall back to defaults # For cases where we don't have specific teams, fall back to defaults
if not old_team: if not old_team:
continue continue
# Create transaction # Create transaction
transaction = Transaction( transaction = Transaction(
id=0, # Will be set by API id=0, # Will be set by API
@ -564,24 +670,26 @@ class TransactionBuilder:
oldteam=old_team, oldteam=old_team,
newteam=new_team, newteam=new_team,
cancelled=False, cancelled=False,
frozen=False frozen=False,
) )
transactions.append(transaction) transactions.append(transaction)
logger.info(f"Created {len(transactions)} transactions for submission with move_id {move_id}") logger.info(
f"Created {len(transactions)} transactions for submission with move_id {move_id}"
)
return transactions return transactions
def clear_moves(self) -> None: def clear_moves(self) -> None:
"""Clear all moves from the transaction builder.""" """Clear all moves from the transaction builder."""
self.moves.clear() self.moves.clear()
logger.info("Cleared all moves from transaction builder") logger.info("Cleared all moves from transaction builder")
@property @property
def is_empty(self) -> bool: def is_empty(self) -> bool:
"""Check if transaction builder has no moves.""" """Check if transaction builder has no moves."""
return len(self.moves) == 0 return len(self.moves) == 0
@property @property
def move_count(self) -> int: def move_count(self) -> int:
"""Get total number of moves in transaction.""" """Get total number of moves in transaction."""
@ -595,17 +703,17 @@ _active_builders: Dict[int, TransactionBuilder] = {}
def get_transaction_builder(user_id: int, team: Team) -> TransactionBuilder: def get_transaction_builder(user_id: int, team: Team) -> TransactionBuilder:
""" """
Get or create a transaction builder for a user. Get or create a transaction builder for a user.
Args: Args:
user_id: Discord user ID user_id: Discord user ID
team: Team object team: Team object
Returns: Returns:
TransactionBuilder instance TransactionBuilder instance
""" """
if user_id not in _active_builders: if user_id not in _active_builders:
_active_builders[user_id] = TransactionBuilder(team, user_id) _active_builders[user_id] = TransactionBuilder(team, user_id)
return _active_builders[user_id] return _active_builders[user_id]
@ -613,4 +721,4 @@ def clear_transaction_builder(user_id: int) -> None:
"""Clear transaction builder for a user.""" """Clear transaction builder for a user."""
if user_id in _active_builders: if user_id in _active_builders:
del _active_builders[user_id] del _active_builders[user_id]
logger.info(f"Cleared transaction builder for user {user_id}") logger.info(f"Cleared transaction builder for user {user_id}")

283
services/transaction_service.py
View File

@ -3,85 +3,91 @@ Transaction service for Discord Bot v2.0
Handles transaction CRUD operations and business logic. Handles transaction CRUD operations and business logic.
""" """
import logging import logging
from typing import Optional, List from typing import Optional, List, Union
from datetime import datetime, UTC from datetime import datetime, UTC
from services.base_service import BaseService from services.base_service import BaseService
from models.transaction import Transaction, RosterValidation from models.transaction import Transaction, RosterValidation
from exceptions import APIException from exceptions import APIException
logger = logging.getLogger(f'{__name__}.TransactionService') logger = logging.getLogger(f"{__name__}.TransactionService")
class TransactionService(BaseService[Transaction]): class TransactionService(BaseService[Transaction]):
"""Service for transaction operations.""" """Service for transaction operations."""
def __init__(self): def __init__(self):
"""Initialize transaction service.""" """Initialize transaction service."""
super().__init__( super().__init__(model_class=Transaction, endpoint="transactions")
model_class=Transaction,
endpoint='transactions'
)
logger.debug("TransactionService initialized") logger.debug("TransactionService initialized")
async def get_team_transactions( async def get_team_transactions(
self, self,
team_abbrev: str, team_abbrev: Union[str, List[str]],
season: int, season: int,
cancelled: Optional[bool] = None, cancelled: Optional[bool] = None,
frozen: Optional[bool] = None, frozen: Optional[bool] = None,
week_start: Optional[int] = None, week_start: Optional[int] = None,
week_end: Optional[int] = None week_end: Optional[int] = None,
) -> List[Transaction]: ) -> List[Transaction]:
""" """
Get transactions for a specific team. Get transactions for a specific team or list of teams.
Args: Args:
team_abbrev: Team abbreviation team_abbrev: Team abbreviation or list of abbreviations (e.g., ["POR", "PORMIL", "PORIL"])
season: Season number season: Season number
cancelled: Filter by cancelled status cancelled: Filter by cancelled status
frozen: Filter by frozen status frozen: Filter by frozen status
week_start: Start week for filtering week_start: Start week for filtering
week_end: End week for filtering week_end: End week for filtering
Returns: Returns:
List of matching transactions List of matching transactions
""" """
try: try:
params = [ params = [("season", str(season))]
('season', str(season)),
('team_abbrev', team_abbrev) # Support single string or list of abbreviations
] if isinstance(team_abbrev, list):
for abbrev in team_abbrev:
params.append(("team_abbrev", abbrev))
else:
params.append(("team_abbrev", team_abbrev))
if cancelled is not None: if cancelled is not None:
params.append(('cancelled', str(cancelled).lower())) params.append(("cancelled", str(cancelled).lower()))
if frozen is not None: if frozen is not None:
params.append(('frozen', str(frozen).lower())) params.append(("frozen", str(frozen).lower()))
if week_start is not None: if week_start is not None:
params.append(('week_start', str(week_start))) params.append(("week_start", str(week_start)))
if week_end is not None: if week_end is not None:
params.append(('week_end', str(week_end))) params.append(("week_end", str(week_end)))
transactions = await self.get_all_items(params=params) transactions = await self.get_all_items(params=params)
# Sort by week, then by moveid # Sort by week, then by moveid
transactions.sort(key=lambda t: (t.week, t.moveid)) transactions.sort(key=lambda t: (t.week, t.moveid))
logger.debug(f"Retrieved {len(transactions)} transactions for {team_abbrev}") logger.debug(
f"Retrieved {len(transactions)} transactions for {team_abbrev}"
)
return transactions return transactions
except Exception as e: except Exception as e:
logger.error(f"Error getting transactions for team {team_abbrev}: {e}") logger.error(f"Error getting transactions for team {team_abbrev}: {e}")
raise APIException(f"Failed to retrieve transactions: {e}") raise APIException(f"Failed to retrieve transactions: {e}")
async def get_pending_transactions(self, team_abbrev: str, season: int) -> List[Transaction]: async def get_pending_transactions(
self, team_abbrev: str, season: int
) -> List[Transaction]:
"""Get pending (future) transactions for a team.""" """Get pending (future) transactions for a team."""
try: try:
# Get current week to filter future transactions # Get current week to filter future transactions
current_data = await self.get_client() current_data = await self.get_client()
current_response = await current_data.get('current') current_response = await current_data.get("current")
current_week = current_response.get('week', 0) if current_response else 0 current_week = current_response.get("week", 0) if current_response else 0
# Get transactions from current week onward # Get transactions from current week onward
return await self.get_team_transactions( return await self.get_team_transactions(
@ -89,105 +95,105 @@ class TransactionService(BaseService[Transaction]):
season, season,
cancelled=False, cancelled=False,
frozen=False, frozen=False,
week_start=current_week week_start=current_week,
) )
except Exception as e: except Exception as e:
logger.warning(f"Could not get current week, returning all non-cancelled/non-frozen transactions: {e}") logger.warning(
f"Could not get current week, returning all non-cancelled/non-frozen transactions: {e}"
)
# Fallback to all non-cancelled/non-frozen if we can't get current week # Fallback to all non-cancelled/non-frozen if we can't get current week
return await self.get_team_transactions( return await self.get_team_transactions(
team_abbrev, team_abbrev, season, cancelled=False, frozen=False
season,
cancelled=False,
frozen=False
) )
async def get_frozen_transactions(self, team_abbrev: str, season: int) -> List[Transaction]: async def get_frozen_transactions(
self, team_abbrev: str, season: int
) -> List[Transaction]:
"""Get frozen (scheduled for processing) transactions for a team.""" """Get frozen (scheduled for processing) transactions for a team."""
return await self.get_team_transactions( return await self.get_team_transactions(team_abbrev, season, frozen=True)
team_abbrev,
season,
frozen=True
)
async def get_processed_transactions( async def get_processed_transactions(
self, self, team_abbrev: str, season: int, recent_weeks: int = 4
team_abbrev: str,
season: int,
recent_weeks: int = 4
) -> List[Transaction]: ) -> List[Transaction]:
"""Get recently processed transactions for a team.""" """Get recently processed transactions for a team."""
# Get current week to limit search # Get current week to limit search
try: try:
current_data = await self.get_client() current_data = await self.get_client()
current_response = await current_data.get('current') current_response = await current_data.get("current")
current_week = current_response.get('week', 0) if current_response else 0 current_week = current_response.get("week", 0) if current_response else 0
week_start = max(1, current_week - recent_weeks) week_start = max(1, current_week - recent_weeks)
# For processed transactions, we need to filter by completed/processed status # For processed transactions, we need to filter by completed/processed status
# Since the API structure doesn't have a processed status, we'll get all non-pending/non-frozen # Since the API structure doesn't have a processed status, we'll get all non-pending/non-frozen
all_transactions = await self.get_team_transactions( all_transactions = await self.get_team_transactions(
team_abbrev, team_abbrev, season, week_start=week_start
season,
week_start=week_start
) )
# Filter for transactions that are neither pending nor frozen (i.e., processed) # Filter for transactions that are neither pending nor frozen (i.e., processed)
processed = [t for t in all_transactions if not t.is_pending and not t.is_frozen and not t.cancelled] processed = [
t
for t in all_transactions
if not t.is_pending and not t.is_frozen and not t.cancelled
]
return processed return processed
except Exception as e: except Exception as e:
logger.warning(f"Could not get current week, using basic query: {e}") logger.warning(f"Could not get current week, using basic query: {e}")
all_transactions = await self.get_team_transactions( all_transactions = await self.get_team_transactions(team_abbrev, season)
team_abbrev,
season
)
# Filter for processed transactions # Filter for processed transactions
processed = [t for t in all_transactions if not t.is_pending and not t.is_frozen and not t.cancelled] processed = [
t
for t in all_transactions
if not t.is_pending and not t.is_frozen and not t.cancelled
]
return processed return processed
async def validate_transaction(self, transaction: Transaction) -> RosterValidation: async def validate_transaction(self, transaction: Transaction) -> RosterValidation:
""" """
Validate a transaction for legality. Validate a transaction for legality.
Args: Args:
transaction: Transaction to validate transaction: Transaction to validate
Returns: Returns:
Validation results with any errors or warnings Validation results with any errors or warnings
""" """
try: try:
validation = RosterValidation(is_legal=True) validation = RosterValidation(is_legal=True)
# Basic validation rules for single-move transactions # Basic validation rules for single-move transactions
if not transaction.player: if not transaction.player:
validation.is_legal = False validation.is_legal = False
validation.errors.append("Transaction has no player") validation.errors.append("Transaction has no player")
if not transaction.oldteam or not transaction.newteam: if not transaction.oldteam or not transaction.newteam:
validation.is_legal = False validation.is_legal = False
validation.errors.append("Transaction missing team information") validation.errors.append("Transaction missing team information")
# Validate player eligibility (basic checks) # Validate player eligibility (basic checks)
if transaction.player and transaction.player.wara < 0: if transaction.player and transaction.player.wara < 0:
validation.warnings.append("Player has negative WARA") validation.warnings.append("Player has negative WARA")
# Add more validation logic as needed # Add more validation logic as needed
# - Roster size limits # - Roster size limits
# - Position requirements # - Position requirements
# - Contract constraints # - Contract constraints
# - etc. # - etc.
logger.debug(f"Validated transaction {transaction.id}: legal={validation.is_legal}") logger.debug(
f"Validated transaction {transaction.id}: legal={validation.is_legal}"
)
return validation return validation
except Exception as e: except Exception as e:
logger.error(f"Error validating transaction {transaction.id}: {e}") logger.error(f"Error validating transaction {transaction.id}: {e}")
# Return failed validation on error # Return failed validation on error
return RosterValidation( return RosterValidation(
is_legal=False, is_legal=False, errors=[f"Validation error: {str(e)}"]
errors=[f"Validation error: {str(e)}"]
) )
async def create_transaction_batch(self, transactions: List[Transaction]) -> List[Transaction]: async def create_transaction_batch(
self, transactions: List[Transaction]
) -> List[Transaction]:
""" """
Create multiple transactions via API POST (for immediate execution). Create multiple transactions via API POST (for immediate execution).
@ -233,15 +239,12 @@ class TransactionService(BaseService[Transaction]):
"season": transaction.season, "season": transaction.season,
"moveid": transaction.moveid, "moveid": transaction.moveid,
"cancelled": transaction.cancelled or False, "cancelled": transaction.cancelled or False,
"frozen": transaction.frozen or False "frozen": transaction.frozen or False,
} }
moves.append(move) moves.append(move)
# Create batch request payload # Create batch request payload
batch_data = { batch_data = {"count": len(moves), "moves": moves}
"count": len(moves),
"moves": moves
}
# POST batch to API # POST batch to API
client = await self.get_client() client = await self.get_client()
@ -249,7 +252,11 @@ class TransactionService(BaseService[Transaction]):
# API returns a string like "2 transactions have been added" # API returns a string like "2 transactions have been added"
# We need to return the original Transaction objects (they won't have IDs assigned by API) # We need to return the original Transaction objects (they won't have IDs assigned by API)
if response and isinstance(response, str) and "transactions have been added" in response: if (
response
and isinstance(response, str)
and "transactions have been added" in response
):
logger.info(f"Successfully created batch: {response}") logger.info(f"Successfully created batch: {response}")
return transactions return transactions
else: else:
@ -276,8 +283,8 @@ class TransactionService(BaseService[Transaction]):
try: try:
# Update transaction status using direct API call to handle bulk updates # Update transaction status using direct API call to handle bulk updates
update_data = { update_data = {
'cancelled': True, "cancelled": True,
'cancelled_at': datetime.now(UTC).isoformat() "cancelled_at": datetime.now(UTC).isoformat(),
} }
# Call API directly since bulk update returns a message string, not a Transaction object # Call API directly since bulk update returns a message string, not a Transaction object
@ -286,12 +293,12 @@ class TransactionService(BaseService[Transaction]):
self.endpoint, self.endpoint,
update_data, update_data,
object_id=transaction_id, object_id=transaction_id,
use_query_params=True use_query_params=True,
) )
# Check if response indicates success # Check if response indicates success
# Response will be a string like "Updated 4 transactions" for bulk updates # Response will be a string like "Updated 4 transactions" for bulk updates
if response and (isinstance(response, str) and 'Updated' in response): if response and (isinstance(response, str) and "Updated" in response):
logger.info(f"Cancelled transaction(s) {transaction_id}: {response}") logger.info(f"Cancelled transaction(s) {transaction_id}: {response}")
return True return True
elif response: elif response:
@ -324,14 +331,14 @@ class TransactionService(BaseService[Transaction]):
client = await self.get_client() client = await self.get_client()
response = await client.patch( response = await client.patch(
self.endpoint, self.endpoint,
{'frozen': False}, {"frozen": False},
object_id=transaction_id, object_id=transaction_id,
use_query_params=True use_query_params=True,
) )
# Check if response indicates success # Check if response indicates success
# Response will be a string like "Updated 4 transactions" for bulk updates # Response will be a string like "Updated 4 transactions" for bulk updates
if response and (isinstance(response, str) and 'Updated' in response): if response and (isinstance(response, str) and "Updated" in response):
logger.info(f"Unfroze transaction(s) {transaction_id}: {response}") logger.info(f"Unfroze transaction(s) {transaction_id}: {response}")
return True return True
elif response: elif response:
@ -347,10 +354,7 @@ class TransactionService(BaseService[Transaction]):
return False return False
async def get_frozen_transactions_by_week( async def get_frozen_transactions_by_week(
self, self, season: int, week_start: int, week_end: int
season: int,
week_start: int,
week_end: int
) -> List[Transaction]: ) -> List[Transaction]:
""" """
Get all frozen transactions for a week range (all teams). Get all frozen transactions for a week range (all teams).
@ -368,25 +372,27 @@ class TransactionService(BaseService[Transaction]):
""" """
try: try:
params = [ params = [
('season', str(season)), ("season", str(season)),
('week_start', str(week_start)), ("week_start", str(week_start)),
('week_end', str(week_end)), ("week_end", str(week_end)),
('frozen', 'true') ("frozen", "true"),
] ]
transactions = await self.get_all_items(params=params) transactions = await self.get_all_items(params=params)
logger.debug(f"Retrieved {len(transactions)} frozen transactions for weeks {week_start}-{week_end}") logger.debug(
f"Retrieved {len(transactions)} frozen transactions for weeks {week_start}-{week_end}"
)
return transactions return transactions
except Exception as e: except Exception as e:
logger.error(f"Error getting frozen transactions for weeks {week_start}-{week_end}: {e}") logger.error(
f"Error getting frozen transactions for weeks {week_start}-{week_end}: {e}"
)
return [] return []
async def get_regular_transactions_by_week( async def get_regular_transactions_by_week(
self, self, season: int, week: int
season: int,
week: int
) -> List[Transaction]: ) -> List[Transaction]:
""" """
Get non-frozen, non-cancelled transactions for a specific week. Get non-frozen, non-cancelled transactions for a specific week.
@ -404,62 +410,66 @@ class TransactionService(BaseService[Transaction]):
""" """
try: try:
params = [ params = [
('season', str(season)), ("season", str(season)),
('week_start', str(week)), ("week_start", str(week)),
('week_end', str(week)), ("week_end", str(week)),
('frozen', 'false'), ("frozen", "false"),
('cancelled', 'false') ("cancelled", "false"),
] ]
transactions = await self.get_all_items(params=params) transactions = await self.get_all_items(params=params)
logger.debug(f"Retrieved {len(transactions)} regular transactions for week {week}") logger.debug(
f"Retrieved {len(transactions)} regular transactions for week {week}"
)
return transactions return transactions
except Exception as e: except Exception as e:
logger.error(f"Error getting regular transactions for week {week}: {e}") logger.error(f"Error getting regular transactions for week {week}: {e}")
return [] return []
async def get_contested_transactions(self, season: int, week: int) -> List[Transaction]: async def get_contested_transactions(
self, season: int, week: int
) -> List[Transaction]:
""" """
Get transactions that may be contested (multiple teams want same player). Get transactions that may be contested (multiple teams want same player).
Args: Args:
season: Season number season: Season number
week: Week number week: Week number
Returns: Returns:
List of potentially contested transactions List of potentially contested transactions
""" """
try: try:
# Get all pending transactions for the week # Get all pending transactions for the week
params = [ params = [
('season', str(season)), ("season", str(season)),
('week', str(week)), ("week", str(week)),
('cancelled', 'false'), ("cancelled", "false"),
('frozen', 'false') ("frozen", "false"),
] ]
transactions = await self.get_all_items(params=params) transactions = await self.get_all_items(params=params)
# Group by players being targeted (simplified contest detection) # Group by players being targeted (simplified contest detection)
player_target_map = {} player_target_map = {}
contested = [] contested = []
for transaction in transactions: for transaction in transactions:
# In the new model, each transaction is a single player move # In the new model, each transaction is a single player move
# Contest occurs when multiple teams try to acquire the same player # Contest occurs when multiple teams try to acquire the same player
if transaction.newteam.abbrev != 'FA': # Not dropping to free agency if transaction.newteam.abbrev != "FA": # Not dropping to free agency
player_name = transaction.player.name.lower() player_name = transaction.player.name.lower()
if player_name not in player_target_map: if player_name not in player_target_map:
player_target_map[player_name] = [] player_target_map[player_name] = []
player_target_map[player_name].append(transaction) player_target_map[player_name].append(transaction)
# Find contested players (wanted by multiple teams) # Find contested players (wanted by multiple teams)
for player_name, player_transactions in player_target_map.items(): for player_name, player_transactions in player_target_map.items():
if len(player_transactions) > 1: if len(player_transactions) > 1:
contested.extend(player_transactions) contested.extend(player_transactions)
# Remove duplicates while preserving order # Remove duplicates while preserving order
seen = set() seen = set()
result = [] result = []
@ -467,8 +477,10 @@ class TransactionService(BaseService[Transaction]):
if transaction.id not in seen: if transaction.id not in seen:
seen.add(transaction.id) seen.add(transaction.id)
result.append(transaction) result.append(transaction)
logger.debug(f"Found {len(result)} potentially contested transactions for week {week}") logger.debug(
f"Found {len(result)} potentially contested transactions for week {week}"
)
return result return result
except Exception as e: except Exception as e:
@ -476,10 +488,7 @@ class TransactionService(BaseService[Transaction]):
return [] return []
async def is_player_in_pending_transaction( async def is_player_in_pending_transaction(
self, self, player_id: int, week: int, season: int
player_id: int,
week: int,
season: int
) -> tuple[bool, Optional[str]]: ) -> tuple[bool, Optional[str]]:
""" """
Check if a player is already in a pending transaction for a specific week. Check if a player is already in a pending transaction for a specific week.
@ -499,10 +508,10 @@ class TransactionService(BaseService[Transaction]):
# Get all pending transactions for the week (all teams) # Get all pending transactions for the week (all teams)
# Use week_start to filter out keepers (week=0) and earlier transactions # Use week_start to filter out keepers (week=0) and earlier transactions
params = [ params = [
('season', str(season)), ("season", str(season)),
('week_start', str(week)), ("week_start", str(week)),
('cancelled', 'false'), ("cancelled", "false"),
('frozen', 'false') ("frozen", "false"),
] ]
transactions = await self.get_all_items(params=params) transactions = await self.get_all_items(params=params)
@ -511,7 +520,9 @@ class TransactionService(BaseService[Transaction]):
for transaction in transactions: for transaction in transactions:
if transaction.player and transaction.player.id == player_id: if transaction.player and transaction.player.id == player_id:
# Found the player in a pending transaction # Found the player in a pending transaction
claiming_team = transaction.newteam.abbrev if transaction.newteam else "Unknown" claiming_team = (
transaction.newteam.abbrev if transaction.newteam else "Unknown"
)
logger.info( logger.info(
f"Player {player_id} already in pending transaction for week {week} " f"Player {player_id} already in pending transaction for week {week} "
f"(claimed by {claiming_team})" f"(claimed by {claiming_team})"
@ -521,11 +532,13 @@ class TransactionService(BaseService[Transaction]):
return False, None return False, None
except Exception as e: except Exception as e:
logger.error(f"Error checking pending transactions for player {player_id}: {e}") logger.error(
f"Error checking pending transactions for player {player_id}: {e}"
)
# On error, allow the transaction (fail open) but log the issue # On error, allow the transaction (fail open) but log the issue
# The freeze task will still catch duplicates if they occur # The freeze task will still catch duplicates if they occur
return False, None return False, None
# Global service instance # Global service instance
transaction_service = TransactionService() transaction_service = TransactionService()

277
tests/test_roster_validation_org_affiliates.py Normal file
View File

@ -0,0 +1,277 @@
"""
Tests for roster validation with organizational affiliate transactions.
Verifies that pending trades involving MiL/IL affiliate teams are correctly
included in roster projections. This was a bug where load_existing_transactions
only queried for the base team abbreviation (e.g., "POR") and missed
transactions involving affiliates ("PORMIL", "PORIL").
Fixes: https://git.manticorum.com/cal/major-domo-v2/issues/49
"""
import pytest
from unittest.mock import AsyncMock, patch
from services.transaction_builder import (
TransactionBuilder,
TransactionMove,
clear_transaction_builder,
)
from models.team import Team, RosterType
from models.player import Player
from models.roster import TeamRoster
from models.transaction import Transaction
from models.current import Current
from tests.factories import PlayerFactory, TeamFactory
def _make_player(id: int, name: str, wara: float = 1.0) -> Player:
"""Create a minimal Player for testing."""
return PlayerFactory.create(id=id, name=name, wara=wara)
def _make_team(id: int, abbrev: str, sname: str, lname: str) -> Team:
"""Create a minimal Team for testing."""
return TeamFactory.create(id=id, abbrev=abbrev, sname=sname, lname=lname)
class TestLoadExistingTransactionsOrgAffiliates:
"""Tests that load_existing_transactions queries all org affiliates."""
@pytest.fixture
def por_team(self):
"""POR major league team."""
return _make_team(100, "POR", "Porters", "Portland Porters")
@pytest.fixture
def por_mil_team(self):
"""PORMIL minor league team."""
return _make_team(101, "PORMIL", "MiL Porters", "Portland Porters MiL")
@pytest.fixture
def por_il_team(self):
"""PORIL injured list team."""
return _make_team(102, "PORIL", "Porter IL", "Portland Porters IL")
@pytest.fixture
def other_team(self):
"""Another team for trades."""
return _make_team(200, "NY", "Yankees", "New York Yankees")
@pytest.fixture
def other_mil_team(self):
"""Another team's MiL."""
return _make_team(201, "NYMIL", "RailRiders", "New York RailRiders")
@pytest.fixture
def mock_roster_at_mil_limit(self, por_team):
"""POR roster with MiL at the 6-player limit.
When MiL is already at limit, adding a player from IL to MiL
should be illegal UNLESS a pending trade clears a spot.
"""
ml_players = [_make_player(1000 + i, f"ML Player {i}") for i in range(24)]
mil_players = [_make_player(2000 + i, f"MiL Player {i}") for i in range(6)]
return TeamRoster(
team_id=por_team.id,
team_abbrev=por_team.abbrev,
week=10,
season=12,
active_players=ml_players,
minor_league_players=mil_players,
)
@pytest.mark.asyncio
async def test_load_existing_transactions_queries_all_org_abbrevs(self, por_team):
"""Verify load_existing_transactions passes ML, MiL, and IL abbreviations.
This is the core fix: the API's team_abbrev filter does exact matching,
so we must query for all organizational affiliates.
"""
with patch(
"services.transaction_builder.transaction_service"
) as mock_tx_service:
mock_tx_service.get_team_transactions = AsyncMock(return_value=[])
builder = TransactionBuilder(por_team, user_id=1, season=12)
await builder.load_existing_transactions(next_week=11)
# Verify the call included all org affiliates
mock_tx_service.get_team_transactions.assert_called_once_with(
team_abbrev=["POR", "PORMIL", "PORIL"],
season=12,
cancelled=False,
week_start=11,
)
@pytest.mark.asyncio
async def test_trade_clearing_mil_spot_allows_il_to_mil_move(
self, por_team, por_mil_team, por_il_team, other_team, mock_roster_at_mil_limit
):
"""Test the exact POR scenario: trade clears MiL spot, allowing IL→MiL move.
Scenario:
- POR has 6 MiL players (at limit)
- POR traded a MiL player to NY for next week (transaction: PORMIL → NY)
- POR wants to move Jeffrey Springs from IL to MiL next week
- Without the fix: validation doesn't see the trade, says "7 players (limit: 6)"
- With the fix: validation sees the trade (-1 MiL), allows the move (6 - 1 + 1 = 6)
"""
# The trade: a PORMIL player going to NY next week
traded_player = _make_player(3000, "Traded Away", wara=1.5)
trade_transaction = Transaction.from_api_data(
{
"id": 999,
"week": 11,
"season": 12,
"moveid": "trade-001",
"player": {
"id": traded_player.id,
"name": traded_player.name,
"wara": traded_player.wara,
"season": 12,
"pos_1": "SP",
},
"oldteam": {
"id": por_mil_team.id,
"abbrev": por_mil_team.abbrev,
"sname": por_mil_team.sname,
"lname": por_mil_team.lname,
"season": 12,
},
"newteam": {
"id": other_team.id,
"abbrev": other_team.abbrev,
"sname": other_team.sname,
"lname": other_team.lname,
"season": 12,
},
"cancelled": False,
"frozen": False,
}
)
# Jeffrey Springs: moving from IL to MiL
springs = _make_player(4000, "Jeffrey Springs", wara=0.8)
with patch(
"services.transaction_builder.roster_service"
) as mock_roster_service:
with patch(
"services.transaction_builder.transaction_service"
) as mock_tx_service:
with patch(
"services.transaction_builder.league_service"
) as mock_league_service:
mock_roster_service.get_current_roster = AsyncMock(
return_value=mock_roster_at_mil_limit
)
# Return the trade transaction when queried with org abbrevs
mock_tx_service.get_team_transactions = AsyncMock(
return_value=[trade_transaction]
)
mock_league_service.get_current_state = AsyncMock(
return_value=Current(week=10, season=12, freeze=False)
)
builder = TransactionBuilder(por_team, user_id=1, season=12)
# Add the IL → MiL move
move = TransactionMove(
player=springs,
from_roster=RosterType.INJURED_LIST,
to_roster=RosterType.MINOR_LEAGUE,
from_team=por_team,
to_team=por_team,
)
await builder.add_move(move, check_pending_transactions=False)
# Validate with next_week to trigger pre-existing transaction loading
validation = await builder.validate_transaction(next_week=11)
# With the trade factored in: 6 (current) - 1 (trade out) + 1 (Springs) = 6
assert validation.is_legal is True
assert validation.minor_league_count == 6
assert validation.pre_existing_transaction_count == 1
@pytest.mark.asyncio
async def test_without_trade_il_to_mil_is_illegal(
self, por_team, mock_roster_at_mil_limit
):
"""Without any pending trade, IL→MiL move should be illegal at MiL limit.
This confirms the baseline behavior: if there's no trade clearing a spot,
validation correctly rejects the move.
"""
springs = _make_player(4000, "Jeffrey Springs", wara=0.8)
with patch(
"services.transaction_builder.roster_service"
) as mock_roster_service:
with patch(
"services.transaction_builder.transaction_service"
) as mock_tx_service:
with patch(
"services.transaction_builder.league_service"
) as mock_league_service:
mock_roster_service.get_current_roster = AsyncMock(
return_value=mock_roster_at_mil_limit
)
mock_tx_service.get_team_transactions = AsyncMock(return_value=[])
mock_league_service.get_current_state = AsyncMock(
return_value=Current(week=10, season=12, freeze=False)
)
builder = TransactionBuilder(por_team, user_id=1, season=12)
move = TransactionMove(
player=springs,
from_roster=RosterType.INJURED_LIST,
to_roster=RosterType.MINOR_LEAGUE,
from_team=por_team,
to_team=por_team,
)
await builder.add_move(move, check_pending_transactions=False)
validation = await builder.validate_transaction(next_week=11)
# Without trade: 6 (current) + 1 (Springs) = 7 > 6 limit
assert validation.is_legal is False
assert validation.minor_league_count == 7
assert "7 players (limit: 6)" in validation.errors[0]
class TestGetTeamTransactionsListSupport:
"""Tests that get_team_transactions accepts a list of abbreviations."""
@pytest.mark.asyncio
async def test_single_string_backwards_compatible(self):
"""Passing a single string still works as before."""
from services.transaction_service import TransactionService
service = TransactionService()
with patch.object(service, "get_all_items", new_callable=AsyncMock) as mock_get:
mock_get.return_value = []
await service.get_team_transactions("POR", 12)
# Should have a single team_abbrev param
call_params = mock_get.call_args[1]["params"]
team_abbrev_params = [p for p in call_params if p[0] == "team_abbrev"]
assert len(team_abbrev_params) == 1
assert team_abbrev_params[0][1] == "POR"
@pytest.mark.asyncio
async def test_list_of_abbreviations(self):
"""Passing a list sends multiple team_abbrev params."""
from services.transaction_service import TransactionService
service = TransactionService()
with patch.object(service, "get_all_items", new_callable=AsyncMock) as mock_get:
mock_get.return_value = []
await service.get_team_transactions(["POR", "PORMIL", "PORIL"], 12)
call_params = mock_get.call_args[1]["params"]
team_abbrev_params = [p for p in call_params if p[0] == "team_abbrev"]
assert len(team_abbrev_params) == 3
assert team_abbrev_params[0][1] == "POR"
assert team_abbrev_params[1][1] == "PORMIL"
assert team_abbrev_params[2][1] == "PORIL"