Merge pull request 'next-release' (#51) from next-release into main
All checks were successful
Build Docker Image / build (push) Successful in 1m0s

Reviewed-on: #51
This commit is contained in:
cal 2026-03-01 20:56:38 +00:00
commit f7a65706a1
3 changed files with 627 additions and 229 deletions

View File

@ -3,6 +3,7 @@ Transaction Builder Service
Handles the complex logic for building multi-move transactions interactively.
"""
import logging
from typing import Dict, List, Optional
from dataclasses import dataclass
@ -18,7 +19,7 @@ from services.transaction_service import transaction_service
from services.league_service import league_service
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
@ -27,53 +28,87 @@ logger = logging.getLogger(f'{__name__}.TransactionBuilder')
@dataclass
class TransactionMove:
"""Individual move within a transaction."""
player: Player
from_roster: RosterType
to_roster: RosterType
from_team: Optional[Team] = None
to_team: Optional[Team] = None
@property
def description(self) -> str:
"""Human readable move description."""
# 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
emoji = ""
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
emoji = ""
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
emoji = "⬆️"
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
emoji = "⬇️"
return f"{emoji} {self.player.name}: {self.from_team.abbrev} (ML) → {self.to_team.abbrev} (MiL)"
elif self.to_roster == RosterType.INJURED_LIST:
# Move to Injured List
emoji = "🏥"
from_desc = "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)"
from_desc = (
"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:
# Return from Injured List
emoji = "💊"
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}"
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:
# Generic move
emoji = "🔄"
from_desc = "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()})"
from_desc = (
"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}"
@dataclass
class RosterValidationResult:
"""Results of roster validation."""
is_legal: bool
major_league_count: int
minor_league_count: int
@ -88,7 +123,7 @@ class RosterValidationResult:
pre_existing_ml_swar_change: float = 0.0
pre_existing_mil_swar_change: float = 0.0
pre_existing_transaction_count: int = 0
@property
def major_league_status(self) -> str:
"""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!)"
else:
return f"✅ Major League: {self.major_league_count}/{self.major_league_limit} (Legal)"
@property
def minor_league_status(self) -> str:
"""Status string for minor league roster."""
@ -124,7 +159,9 @@ class RosterValidationResult:
if self.pre_existing_transaction_count == 0:
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:
return f" **Pre-existing Moves**: {self.pre_existing_transaction_count} scheduled moves (no sWAR impact)"
@ -136,11 +173,11 @@ class RosterValidationResult:
class TransactionBuilder:
"""Interactive transaction builder for complex multi-move transactions."""
def __init__(self, team: Team, user_id: int, season: int = get_config().sba_season):
"""
Initialize transaction builder.
Args:
team: Team making the transaction
user_id: Discord user ID of the GM
@ -151,7 +188,7 @@ class TransactionBuilder:
self.season = season
self.moves: List[TransactionMove] = []
self.created_at = datetime.now(timezone.utc)
# Cache for roster data
self._current_roster: Optional[TeamRoster] = None
self._roster_loaded = False
@ -160,8 +197,10 @@ class TransactionBuilder:
self._existing_transactions: Optional[List[Transaction]] = None
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:
"""Load current roster data for the team."""
if self._roster_loaded:
@ -177,29 +216,42 @@ class TransactionBuilder:
self._roster_loaded = True
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:
return
try:
self._existing_transactions = await transaction_service.get_team_transactions(
team_abbrev=self.team.abbrev,
season=self.season,
cancelled=False,
week_start=next_week
# Include all org affiliates so trades involving MiL/IL teams are captured
base_abbrev = self.team._get_base_abbrev()
org_abbrevs = [base_abbrev, f"{base_abbrev}MIL", f"{base_abbrev}IL"]
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
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:
logger.error(f"Failed to load existing transactions: {e}")
self._existing_transactions = []
self._existing_transactions_loaded = True
async def add_move(
self,
move: TransactionMove,
next_week: Optional[int] = None,
check_pending_transactions: bool = True
check_pending_transactions: bool = True,
) -> tuple[bool, str]:
"""
Add a move to the transaction.
@ -216,14 +268,20 @@ class TransactionBuilder:
# Check if player is already in a move in this transaction builder
existing_move = self.get_move_for_player(move.player.id)
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)
return False, error_msg
# 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
if (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):
if (
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"
logger.warning(error_msg)
return False, error_msg
@ -237,13 +295,15 @@ class TransactionBuilder:
current_state = await league_service.get_current_state()
next_week = (current_state.week + 1) if current_state else 1
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
is_pending, claiming_team = await transaction_service.is_player_in_pending_transaction(
player_id=move.player.id,
week=next_week,
season=self.season
is_pending, claiming_team = (
await transaction_service.is_player_in_pending_transaction(
player_id=move.player.id, week=next_week, season=self.season
)
)
if is_pending:
@ -254,34 +314,36 @@ class TransactionBuilder:
self.moves.append(move)
logger.info(f"Added move: {move.description}")
return True, ""
def remove_move(self, player_id: int) -> bool:
"""
Remove a move for a specific player.
Args:
player_id: ID of player to remove move for
Returns:
True if move was removed
"""
original_count = len(self.moves)
self.moves = [move for move in self.moves if move.player.id != player_id]
removed = len(self.moves) < original_count
if removed:
logger.info(f"Removed move for player {player_id}")
return removed
def get_move_for_player(self, player_id: int) -> Optional[TransactionMove]:
"""Get the move for a specific player if it exists."""
for move in self.moves:
if move.player.id == player_id:
return move
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.
@ -296,17 +358,17 @@ class TransactionBuilder:
# Load existing transactions if next_week is provided
if next_week is not None:
await self.load_existing_transactions(next_week)
if not self._current_roster:
return RosterValidationResult(
is_legal=False,
major_league_count=0,
minor_league_count=0,
minor_league_count=0,
warnings=[],
errors=["Could not load current roster data"],
suggestions=[]
suggestions=[],
)
# Calculate roster changes from moves
ml_changes = 0
mil_changes = 0
@ -315,8 +377,12 @@ class TransactionBuilder:
suggestions = []
# Calculate current sWAR for each roster
current_ml_swar = sum(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)
current_ml_swar = sum(
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
ml_swar_changes = 0.0
@ -371,46 +437,66 @@ class TransactionBuilder:
for move in self.moves:
# Log move being processed for diagnostics
logger.debug(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}")
logger.debug(
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
if move.from_roster == RosterType.MAJOR_LEAGUE:
ml_changes -= 1
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:
mil_changes -= 1
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
if move.to_roster == RosterType.MAJOR_LEAGUE:
ml_changes += 1
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:
mil_changes += 1
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
# Calculate projected roster sizes and sWAR
# Only Major League players count toward ML roster limit (IL and MiL are separate)
current_ml_size = len(self._current_roster.active_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(f"🔍 VALIDATION: Changes calculated - ml_changes:{ml_changes}, mil_changes:{mil_changes}")
logger.debug(
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_mil_size = current_mil_size + mil_changes
projected_ml_swar = current_ml_swar + ml_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(f"🔍 VALIDATION: Projected sWAR - ML:{projected_ml_swar:.2f}, MiL:{projected_mil_swar:.2f}")
logger.debug(
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
config = get_config()
try:
@ -432,38 +518,54 @@ class TransactionBuilder:
else:
ml_limit = config.ml_roster_limit_late
mil_limit = config.mil_roster_limit_late
# Validate roster limits
is_legal = True
if projected_ml_size > ml_limit:
is_legal = False
errors.append(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")
errors.append(
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:
is_legal = False
errors.append("Cannot have negative players on Major League roster")
if projected_mil_size > mil_limit:
is_legal = False
errors.append(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")
errors.append(
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:
is_legal = False
errors.append("Cannot have negative players on Minor League roster")
# Validate sWAR cap
# 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:
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
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
if not self.moves:
suggestions.append("Add player moves to build your transaction")
return RosterValidationResult(
is_legal=is_legal,
major_league_count=projected_ml_size,
@ -478,10 +580,12 @@ class TransactionBuilder:
major_league_swar_cap=team_swar_cap,
pre_existing_ml_swar_change=pre_existing_ml_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.
@ -504,11 +608,13 @@ class TransactionBuilder:
validation = await self.validate_transaction()
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 = []
move_id = f"Season-{self.season:03d}-Week-{week:02d}-{int(self.created_at.timestamp())}"
# Create FA team for drops using config value
config = get_config()
fa_team = Team(
@ -516,9 +622,9 @@ class TransactionBuilder:
abbrev="FA",
sname="Free Agents",
lname="Free Agency",
season=self.season
) # type: ignore
season=self.season,
) # type: ignore
for move in self.moves:
# Determine old and new teams based on roster locations
# 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
if not old_team:
continue
# Create transaction
transaction = Transaction(
id=0, # Will be set by API
@ -564,24 +670,26 @@ class TransactionBuilder:
oldteam=old_team,
newteam=new_team,
cancelled=False,
frozen=False
frozen=False,
)
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
def clear_moves(self) -> None:
"""Clear all moves from the transaction builder."""
self.moves.clear()
logger.info("Cleared all moves from transaction builder")
@property
def is_empty(self) -> bool:
"""Check if transaction builder has no moves."""
return len(self.moves) == 0
@property
def move_count(self) -> int:
"""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:
"""
Get or create a transaction builder for a user.
Args:
user_id: Discord user ID
team: Team object
Returns:
TransactionBuilder instance
"""
if user_id not in _active_builders:
_active_builders[user_id] = TransactionBuilder(team, 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."""
if user_id in _active_builders:
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}")

View File

@ -3,85 +3,91 @@ Transaction service for Discord Bot v2.0
Handles transaction CRUD operations and business logic.
"""
import logging
from typing import Optional, List
from typing import Optional, List, Union
from datetime import datetime, UTC
from services.base_service import BaseService
from models.transaction import Transaction, RosterValidation
from exceptions import APIException
logger = logging.getLogger(f'{__name__}.TransactionService')
logger = logging.getLogger(f"{__name__}.TransactionService")
class TransactionService(BaseService[Transaction]):
"""Service for transaction operations."""
def __init__(self):
"""Initialize transaction service."""
super().__init__(
model_class=Transaction,
endpoint='transactions'
)
super().__init__(model_class=Transaction, endpoint="transactions")
logger.debug("TransactionService initialized")
async def get_team_transactions(
self,
team_abbrev: str,
self,
team_abbrev: Union[str, List[str]],
season: int,
cancelled: Optional[bool] = None,
frozen: Optional[bool] = None,
week_start: Optional[int] = None,
week_end: Optional[int] = None
week_end: Optional[int] = None,
) -> List[Transaction]:
"""
Get transactions for a specific team.
Get transactions for a specific team or list of teams.
Args:
team_abbrev: Team abbreviation
team_abbrev: Team abbreviation or list of abbreviations (e.g., ["POR", "PORMIL", "PORIL"])
season: Season number
cancelled: Filter by cancelled status
frozen: Filter by frozen status
week_start: Start week for filtering
week_end: End week for filtering
Returns:
List of matching transactions
"""
try:
params = [
('season', str(season)),
('team_abbrev', team_abbrev)
]
params = [("season", str(season))]
# 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:
params.append(('cancelled', str(cancelled).lower()))
params.append(("cancelled", str(cancelled).lower()))
if frozen is not None:
params.append(('frozen', str(frozen).lower()))
params.append(("frozen", str(frozen).lower()))
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:
params.append(('week_end', str(week_end)))
params.append(("week_end", str(week_end)))
transactions = await self.get_all_items(params=params)
# Sort by week, then by 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
except Exception as e:
logger.error(f"Error getting transactions for team {team_abbrev}: {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."""
try:
# Get current week to filter future transactions
current_data = await self.get_client()
current_response = await current_data.get('current')
current_week = current_response.get('week', 0) if current_response else 0
current_response = await current_data.get("current")
current_week = current_response.get("week", 0) if current_response else 0
# Get transactions from current week onward
return await self.get_team_transactions(
@ -89,105 +95,105 @@ class TransactionService(BaseService[Transaction]):
season,
cancelled=False,
frozen=False,
week_start=current_week
week_start=current_week,
)
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
return await self.get_team_transactions(
team_abbrev,
season,
cancelled=False,
frozen=False
team_abbrev, 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."""
return await self.get_team_transactions(
team_abbrev,
season,
frozen=True
)
return await self.get_team_transactions(team_abbrev, season, frozen=True)
async def get_processed_transactions(
self,
team_abbrev: str,
season: int,
recent_weeks: int = 4
self, team_abbrev: str, season: int, recent_weeks: int = 4
) -> List[Transaction]:
"""Get recently processed transactions for a team."""
# Get current week to limit search
try:
current_data = await self.get_client()
current_response = await current_data.get('current')
current_week = current_response.get('week', 0) if current_response else 0
current_response = await current_data.get("current")
current_week = current_response.get("week", 0) if current_response else 0
week_start = max(1, current_week - recent_weeks)
# 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
all_transactions = await self.get_team_transactions(
team_abbrev,
season,
week_start=week_start
team_abbrev, season, week_start=week_start
)
# 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
except Exception as e:
logger.warning(f"Could not get current week, using basic query: {e}")
all_transactions = await self.get_team_transactions(
team_abbrev,
season
)
all_transactions = await self.get_team_transactions(team_abbrev, season)
# 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
async def validate_transaction(self, transaction: Transaction) -> RosterValidation:
"""
Validate a transaction for legality.
Args:
transaction: Transaction to validate
Returns:
Validation results with any errors or warnings
"""
try:
validation = RosterValidation(is_legal=True)
# Basic validation rules for single-move transactions
if not transaction.player:
validation.is_legal = False
validation.errors.append("Transaction has no player")
if not transaction.oldteam or not transaction.newteam:
validation.is_legal = False
validation.errors.append("Transaction missing team information")
# Validate player eligibility (basic checks)
if transaction.player and transaction.player.wara < 0:
validation.warnings.append("Player has negative WARA")
# Add more validation logic as needed
# - Roster size limits
# - Position requirements
# - Contract constraints
# - 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
except Exception as e:
logger.error(f"Error validating transaction {transaction.id}: {e}")
# Return failed validation on error
return RosterValidation(
is_legal=False,
errors=[f"Validation error: {str(e)}"]
is_legal=False, 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).
@ -233,15 +239,12 @@ class TransactionService(BaseService[Transaction]):
"season": transaction.season,
"moveid": transaction.moveid,
"cancelled": transaction.cancelled or False,
"frozen": transaction.frozen or False
"frozen": transaction.frozen or False,
}
moves.append(move)
# Create batch request payload
batch_data = {
"count": len(moves),
"moves": moves
}
batch_data = {"count": len(moves), "moves": moves}
# POST batch to API
client = await self.get_client()
@ -249,7 +252,11 @@ class TransactionService(BaseService[Transaction]):
# 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)
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}")
return transactions
else:
@ -276,8 +283,8 @@ class TransactionService(BaseService[Transaction]):
try:
# Update transaction status using direct API call to handle bulk updates
update_data = {
'cancelled': True,
'cancelled_at': datetime.now(UTC).isoformat()
"cancelled": True,
"cancelled_at": datetime.now(UTC).isoformat(),
}
# Call API directly since bulk update returns a message string, not a Transaction object
@ -286,12 +293,12 @@ class TransactionService(BaseService[Transaction]):
self.endpoint,
update_data,
object_id=transaction_id,
use_query_params=True
use_query_params=True,
)
# Check if response indicates success
# 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}")
return True
elif response:
@ -324,14 +331,14 @@ class TransactionService(BaseService[Transaction]):
client = await self.get_client()
response = await client.patch(
self.endpoint,
{'frozen': False},
{"frozen": False},
object_id=transaction_id,
use_query_params=True
use_query_params=True,
)
# Check if response indicates success
# 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}")
return True
elif response:
@ -347,10 +354,7 @@ class TransactionService(BaseService[Transaction]):
return False
async def get_frozen_transactions_by_week(
self,
season: int,
week_start: int,
week_end: int
self, season: int, week_start: int, week_end: int
) -> List[Transaction]:
"""
Get all frozen transactions for a week range (all teams).
@ -368,25 +372,27 @@ class TransactionService(BaseService[Transaction]):
"""
try:
params = [
('season', str(season)),
('week_start', str(week_start)),
('week_end', str(week_end)),
('frozen', 'true')
("season", str(season)),
("week_start", str(week_start)),
("week_end", str(week_end)),
("frozen", "true"),
]
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
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 []
async def get_regular_transactions_by_week(
self,
season: int,
week: int
self, season: int, week: int
) -> List[Transaction]:
"""
Get non-frozen, non-cancelled transactions for a specific week.
@ -404,62 +410,66 @@ class TransactionService(BaseService[Transaction]):
"""
try:
params = [
('season', str(season)),
('week_start', str(week)),
('week_end', str(week)),
('frozen', 'false'),
('cancelled', 'false')
("season", str(season)),
("week_start", str(week)),
("week_end", str(week)),
("frozen", "false"),
("cancelled", "false"),
]
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
except Exception as e:
logger.error(f"Error getting regular transactions for week {week}: {e}")
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).
Args:
season: Season number
week: Week number
Returns:
List of potentially contested transactions
"""
try:
# Get all pending transactions for the week
params = [
('season', str(season)),
('week', str(week)),
('cancelled', 'false'),
('frozen', 'false')
("season", str(season)),
("week", str(week)),
("cancelled", "false"),
("frozen", "false"),
]
transactions = await self.get_all_items(params=params)
# Group by players being targeted (simplified contest detection)
player_target_map = {}
contested = []
for transaction in transactions:
# In the new model, each transaction is a single player move
# 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()
if player_name not in player_target_map:
player_target_map[player_name] = []
player_target_map[player_name].append(transaction)
# Find contested players (wanted by multiple teams)
for player_name, player_transactions in player_target_map.items():
if len(player_transactions) > 1:
contested.extend(player_transactions)
# Remove duplicates while preserving order
seen = set()
result = []
@ -467,8 +477,10 @@ class TransactionService(BaseService[Transaction]):
if transaction.id not in seen:
seen.add(transaction.id)
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
except Exception as e:
@ -476,10 +488,7 @@ class TransactionService(BaseService[Transaction]):
return []
async def is_player_in_pending_transaction(
self,
player_id: int,
week: int,
season: int
self, player_id: int, week: int, season: int
) -> tuple[bool, Optional[str]]:
"""
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)
# Use week_start to filter out keepers (week=0) and earlier transactions
params = [
('season', str(season)),
('week_start', str(week)),
('cancelled', 'false'),
('frozen', 'false')
("season", str(season)),
("week_start", str(week)),
("cancelled", "false"),
("frozen", "false"),
]
transactions = await self.get_all_items(params=params)
@ -511,7 +520,9 @@ class TransactionService(BaseService[Transaction]):
for transaction in transactions:
if transaction.player and transaction.player.id == player_id:
# 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(
f"Player {player_id} already in pending transaction for week {week} "
f"(claimed by {claiming_team})"
@ -521,11 +532,13 @@ class TransactionService(BaseService[Transaction]):
return False, None
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
# The freeze task will still catch duplicates if they occur
return False, None
# Global service instance
transaction_service = TransactionService()
transaction_service = TransactionService()

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"