fix: refresh roster data before validation to prevent stale cache
TransactionBuilder cached roster data indefinitely via _roster_loaded flag, causing validation to use stale counts when builders persisted across multiple /ilmove invocations. Now validate_transaction() always fetches fresh roster data. Also adds dependency injection for roster_service to improve testability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Cal Corum
parent
6e4191f677
commit
c70669d474
@ -174,7 +174,13 @@ 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):
|
||||
def __init__(
|
||||
self,
|
||||
team: Team,
|
||||
user_id: int,
|
||||
season: int = get_config().sba_season,
|
||||
roster_svc=None,
|
||||
):
|
||||
"""
|
||||
Initialize transaction builder.
|
||||
|
||||
@ -182,12 +188,14 @@ class TransactionBuilder:
|
||||
team: Team making the transaction
|
||||
user_id: Discord user ID of the GM
|
||||
season: Season number
|
||||
roster_svc: RosterService instance (defaults to global roster_service)
|
||||
"""
|
||||
self.team = team
|
||||
self.user_id = user_id
|
||||
self.season = season
|
||||
self.moves: List[TransactionMove] = []
|
||||
self.created_at = datetime.now(timezone.utc)
|
||||
self._roster_svc = roster_svc or roster_service
|
||||
|
||||
# Cache for roster data
|
||||
self._current_roster: Optional[TeamRoster] = None
|
||||
@ -201,13 +209,19 @@ class TransactionBuilder:
|
||||
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:
|
||||
async def load_roster_data(self, force_refresh: bool = False) -> None:
|
||||
"""Load current roster data for the team.
|
||||
|
||||
Args:
|
||||
force_refresh: If True, bypass cache and fetch fresh data from API.
|
||||
"""
|
||||
if self._roster_loaded and not force_refresh:
|
||||
return
|
||||
|
||||
try:
|
||||
self._current_roster = await roster_service.get_current_roster(self.team.id)
|
||||
self._current_roster = await self._roster_svc.get_current_roster(
|
||||
self.team.id
|
||||
)
|
||||
self._roster_loaded = True
|
||||
logger.debug(f"Loaded roster data for team {self.team.abbrev}")
|
||||
except Exception as e:
|
||||
@ -353,7 +367,9 @@ class TransactionBuilder:
|
||||
Returns:
|
||||
RosterValidationResult with validation details
|
||||
"""
|
||||
await self.load_roster_data()
|
||||
# Always refresh roster data before validation to prevent stale cache
|
||||
# from causing incorrect roster counts (e.g. after a previous /ilmove submission)
|
||||
await self.load_roster_data(force_refresh=True)
|
||||
|
||||
# Load existing transactions if next_week is provided
|
||||
if next_week is not None:
|
||||
|
||||
@ -3,6 +3,7 @@ Tests for TransactionBuilder service
|
||||
|
||||
Validates transaction building, roster validation, and move management.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
@ -12,7 +13,7 @@ from services.transaction_builder import (
|
||||
RosterType,
|
||||
RosterValidationResult,
|
||||
get_transaction_builder,
|
||||
clear_transaction_builder
|
||||
clear_transaction_builder,
|
||||
)
|
||||
from models.team import Team
|
||||
from models.player import Player
|
||||
@ -28,23 +29,17 @@ class TestTransactionBuilder:
|
||||
"""Create a mock team for testing."""
|
||||
return Team(
|
||||
id=499,
|
||||
abbrev='WV',
|
||||
sname='Black Bears',
|
||||
lname='West Virginia Black Bears',
|
||||
abbrev="WV",
|
||||
sname="Black Bears",
|
||||
lname="West Virginia Black Bears",
|
||||
season=12,
|
||||
salary_cap=50.0 # Set high cap to avoid sWAR validation failures in roster tests
|
||||
salary_cap=50.0, # Set high cap to avoid sWAR validation failures in roster tests
|
||||
)
|
||||
|
||||
@pytest.fixture
|
||||
def mock_player(self):
|
||||
"""Create a mock player for testing."""
|
||||
return Player(
|
||||
id=12472,
|
||||
name='Test Player',
|
||||
wara=2.5,
|
||||
season=12,
|
||||
pos_1='OF'
|
||||
)
|
||||
return Player(id=12472, name="Test Player", wara=2.5, season=12, pos_1="OF")
|
||||
|
||||
@pytest.fixture
|
||||
def mock_roster(self):
|
||||
@ -52,67 +47,80 @@ class TestTransactionBuilder:
|
||||
# Create roster players
|
||||
ml_players = []
|
||||
for i in range(24): # 24 ML players (under limit)
|
||||
ml_players.append(Player(
|
||||
id=1000 + i,
|
||||
name=f'ML Player {i}',
|
||||
wara=1.5,
|
||||
season=12,
|
||||
team_id=499,
|
||||
team=None,
|
||||
image=None,
|
||||
image2=None,
|
||||
vanity_card=None,
|
||||
headshot=None,
|
||||
pos_1='OF',
|
||||
pitcher_injury=None,
|
||||
injury_rating=None,
|
||||
il_return=None,
|
||||
demotion_week=None,
|
||||
last_game=None,
|
||||
last_game2=None,
|
||||
strat_code=None,
|
||||
bbref_id=None,
|
||||
sbaplayer=None
|
||||
))
|
||||
ml_players.append(
|
||||
Player(
|
||||
id=1000 + i,
|
||||
name=f"ML Player {i}",
|
||||
wara=1.5,
|
||||
season=12,
|
||||
team_id=499,
|
||||
team=None,
|
||||
image=None,
|
||||
image2=None,
|
||||
vanity_card=None,
|
||||
headshot=None,
|
||||
pos_1="OF",
|
||||
pitcher_injury=None,
|
||||
injury_rating=None,
|
||||
il_return=None,
|
||||
demotion_week=None,
|
||||
last_game=None,
|
||||
last_game2=None,
|
||||
strat_code=None,
|
||||
bbref_id=None,
|
||||
sbaplayer=None,
|
||||
)
|
||||
)
|
||||
|
||||
mil_players = []
|
||||
for i in range(6): # 6 MiL players (at limit)
|
||||
mil_players.append(Player(
|
||||
id=2000 + i,
|
||||
name=f'MiL Player {i}',
|
||||
wara=0.5,
|
||||
season=12,
|
||||
team_id=499,
|
||||
team=None,
|
||||
image=None,
|
||||
image2=None,
|
||||
vanity_card=None,
|
||||
headshot=None,
|
||||
pos_1='OF',
|
||||
pitcher_injury=None,
|
||||
injury_rating=None,
|
||||
il_return=None,
|
||||
demotion_week=None,
|
||||
last_game=None,
|
||||
last_game2=None,
|
||||
strat_code=None,
|
||||
bbref_id=None,
|
||||
sbaplayer=None
|
||||
))
|
||||
mil_players.append(
|
||||
Player(
|
||||
id=2000 + i,
|
||||
name=f"MiL Player {i}",
|
||||
wara=0.5,
|
||||
season=12,
|
||||
team_id=499,
|
||||
team=None,
|
||||
image=None,
|
||||
image2=None,
|
||||
vanity_card=None,
|
||||
headshot=None,
|
||||
pos_1="OF",
|
||||
pitcher_injury=None,
|
||||
injury_rating=None,
|
||||
il_return=None,
|
||||
demotion_week=None,
|
||||
last_game=None,
|
||||
last_game2=None,
|
||||
strat_code=None,
|
||||
bbref_id=None,
|
||||
sbaplayer=None,
|
||||
)
|
||||
)
|
||||
|
||||
return TeamRoster(
|
||||
team_id=499,
|
||||
team_abbrev='WV',
|
||||
team_abbrev="WV",
|
||||
week=10,
|
||||
season=12,
|
||||
active_players=ml_players,
|
||||
minor_league_players=mil_players
|
||||
minor_league_players=mil_players,
|
||||
)
|
||||
|
||||
@pytest.fixture
|
||||
def builder(self, mock_team):
|
||||
"""Create a TransactionBuilder for testing."""
|
||||
return TransactionBuilder(mock_team, user_id=123456789, season=12)
|
||||
def mock_roster_service(self, mock_roster):
|
||||
"""Create a mock roster service that returns mock_roster."""
|
||||
svc = AsyncMock()
|
||||
svc.get_current_roster.return_value = mock_roster
|
||||
return svc
|
||||
|
||||
@pytest.fixture
|
||||
def builder(self, mock_team, mock_roster_service):
|
||||
"""Create a TransactionBuilder for testing with injected roster service."""
|
||||
return TransactionBuilder(
|
||||
mock_team, user_id=123456789, season=12, roster_svc=mock_roster_service
|
||||
)
|
||||
|
||||
def test_builder_initialization(self, builder, mock_team):
|
||||
"""Test transaction builder initialization."""
|
||||
@ -130,11 +138,13 @@ class TestTransactionBuilder:
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
to_team=builder.team,
|
||||
)
|
||||
|
||||
# Skip pending transaction check for unit tests
|
||||
success, error_message = await builder.add_move(move, check_pending_transactions=False)
|
||||
success, error_message = await builder.add_move(
|
||||
move, check_pending_transactions=False
|
||||
)
|
||||
|
||||
assert success is True
|
||||
assert error_message == ""
|
||||
@ -149,18 +159,22 @@ class TestTransactionBuilder:
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
to_team=builder.team,
|
||||
)
|
||||
|
||||
move2 = TransactionMove(
|
||||
player=mock_player,
|
||||
from_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_roster=RosterType.FREE_AGENCY,
|
||||
from_team=builder.team
|
||||
from_team=builder.team,
|
||||
)
|
||||
|
||||
success1, error_message1 = await builder.add_move(move1, check_pending_transactions=False)
|
||||
success2, error_message2 = await builder.add_move(move2, check_pending_transactions=False)
|
||||
success1, error_message1 = await builder.add_move(
|
||||
move1, check_pending_transactions=False
|
||||
)
|
||||
success2, error_message2 = await builder.add_move(
|
||||
move2, check_pending_transactions=False
|
||||
)
|
||||
|
||||
assert success1 is True
|
||||
assert error_message1 == ""
|
||||
@ -176,10 +190,12 @@ class TestTransactionBuilder:
|
||||
from_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_roster=RosterType.MAJOR_LEAGUE, # Same roster
|
||||
from_team=builder.team,
|
||||
to_team=builder.team # Same team - should fail when roster is also same
|
||||
to_team=builder.team, # Same team - should fail when roster is also same
|
||||
)
|
||||
|
||||
success, error_message = await builder.add_move(move, check_pending_transactions=False)
|
||||
success, error_message = await builder.add_move(
|
||||
move, check_pending_transactions=False
|
||||
)
|
||||
|
||||
assert success is False
|
||||
assert "already in that location" in error_message
|
||||
@ -187,17 +203,21 @@ class TestTransactionBuilder:
|
||||
assert builder.is_empty is True
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_move_same_team_different_roster_succeeds(self, builder, mock_player):
|
||||
async def test_add_move_same_team_different_roster_succeeds(
|
||||
self, builder, mock_player
|
||||
):
|
||||
"""Test that adding a move where teams are same but rosters are different succeeds."""
|
||||
move = TransactionMove(
|
||||
player=mock_player,
|
||||
from_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_roster=RosterType.MINOR_LEAGUE, # Different roster
|
||||
from_team=builder.team,
|
||||
to_team=builder.team # Same team - should succeed when rosters differ
|
||||
to_team=builder.team, # Same team - should succeed when rosters differ
|
||||
)
|
||||
|
||||
success, error_message = await builder.add_move(move, check_pending_transactions=False)
|
||||
success, error_message = await builder.add_move(
|
||||
move, check_pending_transactions=False
|
||||
)
|
||||
|
||||
assert success is True
|
||||
assert error_message == ""
|
||||
@ -208,11 +228,7 @@ class TestTransactionBuilder:
|
||||
async def test_add_move_different_teams_succeeds(self, builder, mock_player):
|
||||
"""Test that adding a move where from_team and to_team are different succeeds."""
|
||||
other_team = Team(
|
||||
id=500,
|
||||
abbrev='NY',
|
||||
sname='Mets',
|
||||
lname='New York Mets',
|
||||
season=12
|
||||
id=500, abbrev="NY", sname="Mets", lname="New York Mets", season=12
|
||||
)
|
||||
|
||||
move = TransactionMove(
|
||||
@ -220,10 +236,12 @@ class TestTransactionBuilder:
|
||||
from_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
from_team=other_team,
|
||||
to_team=builder.team
|
||||
to_team=builder.team,
|
||||
)
|
||||
|
||||
success, error_message = await builder.add_move(move, check_pending_transactions=False)
|
||||
success, error_message = await builder.add_move(
|
||||
move, check_pending_transactions=False
|
||||
)
|
||||
|
||||
assert success is True
|
||||
assert error_message == ""
|
||||
@ -239,10 +257,12 @@ class TestTransactionBuilder:
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
from_team=None,
|
||||
to_team=builder.team
|
||||
to_team=builder.team,
|
||||
)
|
||||
|
||||
success1, error_message1 = await builder.add_move(move1, check_pending_transactions=False)
|
||||
success1, error_message1 = await builder.add_move(
|
||||
move1, check_pending_transactions=False
|
||||
)
|
||||
assert success1 is True
|
||||
assert error_message1 == ""
|
||||
|
||||
@ -250,11 +270,7 @@ class TestTransactionBuilder:
|
||||
|
||||
# Create different player for second test
|
||||
other_player = Player(
|
||||
id=12473,
|
||||
name='Other Player',
|
||||
wara=1.5,
|
||||
season=12,
|
||||
pos_1='OF'
|
||||
id=12473, name="Other Player", wara=1.5, season=12, pos_1="OF"
|
||||
)
|
||||
|
||||
# From team to FA (to_team=None)
|
||||
@ -263,10 +279,12 @@ class TestTransactionBuilder:
|
||||
from_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_roster=RosterType.FREE_AGENCY,
|
||||
from_team=builder.team,
|
||||
to_team=None
|
||||
to_team=None,
|
||||
)
|
||||
|
||||
success2, error_message2 = await builder.add_move(move2, check_pending_transactions=False)
|
||||
success2, error_message2 = await builder.add_move(
|
||||
move2, check_pending_transactions=False
|
||||
)
|
||||
assert success2 is True
|
||||
assert error_message2 == ""
|
||||
|
||||
@ -277,7 +295,7 @@ class TestTransactionBuilder:
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
to_team=builder.team,
|
||||
)
|
||||
|
||||
success, _ = await builder.add_move(move, check_pending_transactions=False)
|
||||
@ -304,7 +322,7 @@ class TestTransactionBuilder:
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
to_team=builder.team,
|
||||
)
|
||||
|
||||
await builder.add_move(move, check_pending_transactions=False)
|
||||
@ -322,7 +340,7 @@ class TestTransactionBuilder:
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
to_team=builder.team,
|
||||
)
|
||||
|
||||
success, _ = await builder.add_move(move, check_pending_transactions=False)
|
||||
@ -335,80 +353,78 @@ class TestTransactionBuilder:
|
||||
assert builder.is_empty is True
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validate_transaction_no_roster(self, builder):
|
||||
async def test_validate_transaction_no_roster(self, mock_team):
|
||||
"""Test validation when roster data cannot be loaded."""
|
||||
with patch.object(builder, '_current_roster', None):
|
||||
with patch.object(builder, '_roster_loaded', True):
|
||||
validation = await builder.validate_transaction()
|
||||
failing_svc = AsyncMock()
|
||||
failing_svc.get_current_roster.return_value = None
|
||||
builder = TransactionBuilder(
|
||||
mock_team, user_id=123456789, season=12, roster_svc=failing_svc
|
||||
)
|
||||
|
||||
assert validation.is_legal is False
|
||||
assert len(validation.errors) == 1
|
||||
assert "Could not load current roster data" in validation.errors[0]
|
||||
validation = await builder.validate_transaction()
|
||||
|
||||
assert validation.is_legal is False
|
||||
assert len(validation.errors) == 1
|
||||
assert "Could not load current roster data" in validation.errors[0]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validate_transaction_legal(self, builder, mock_roster, mock_player):
|
||||
async def test_validate_transaction_legal(self, builder, mock_player):
|
||||
"""Test validation of a legal transaction."""
|
||||
with patch.object(builder, '_current_roster', mock_roster):
|
||||
with patch.object(builder, '_roster_loaded', True):
|
||||
# Add a move that keeps roster under limit (24 -> 25)
|
||||
move = TransactionMove(
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
)
|
||||
success, _ = await builder.add_move(move, check_pending_transactions=False)
|
||||
assert success
|
||||
# Add a move that keeps roster under limit (24 -> 25)
|
||||
move = TransactionMove(
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team,
|
||||
)
|
||||
success, _ = await builder.add_move(move, check_pending_transactions=False)
|
||||
assert success
|
||||
|
||||
validation = await builder.validate_transaction()
|
||||
validation = await builder.validate_transaction()
|
||||
|
||||
assert validation.is_legal is True
|
||||
assert validation.major_league_count == 25 # 24 + 1
|
||||
assert len(validation.errors) == 0
|
||||
assert validation.is_legal is True
|
||||
assert validation.major_league_count == 25 # 24 + 1
|
||||
assert len(validation.errors) == 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validate_transaction_over_limit(self, builder, mock_roster):
|
||||
async def test_validate_transaction_over_limit(self, builder):
|
||||
"""Test validation when transaction would exceed roster limit."""
|
||||
with patch.object(builder, '_current_roster', mock_roster):
|
||||
with patch.object(builder, '_roster_loaded', True):
|
||||
# Add 3 players to exceed limit (24 + 3 = 27 > 26)
|
||||
for i in range(3):
|
||||
player = Player(
|
||||
id=3000 + i,
|
||||
name=f'New Player {i}',
|
||||
wara=1.0,
|
||||
season=12,
|
||||
pos_1='OF'
|
||||
)
|
||||
move = TransactionMove(
|
||||
player=player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
)
|
||||
success, _ = await builder.add_move(move, check_pending_transactions=False)
|
||||
assert success
|
||||
# Add 3 players to exceed limit (24 + 3 = 27 > 26)
|
||||
for i in range(3):
|
||||
player = Player(
|
||||
id=3000 + i,
|
||||
name=f"New Player {i}",
|
||||
wara=1.0,
|
||||
season=12,
|
||||
pos_1="OF",
|
||||
)
|
||||
move = TransactionMove(
|
||||
player=player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team,
|
||||
)
|
||||
success, _ = await builder.add_move(move, check_pending_transactions=False)
|
||||
assert success
|
||||
|
||||
validation = await builder.validate_transaction()
|
||||
validation = await builder.validate_transaction()
|
||||
|
||||
assert validation.is_legal is False
|
||||
assert validation.major_league_count == 27 # 24 + 3
|
||||
assert len(validation.errors) == 1
|
||||
assert "27 players (limit: 26)" in validation.errors[0]
|
||||
assert len(validation.suggestions) == 1
|
||||
assert "Drop 1 ML player" in validation.suggestions[0]
|
||||
assert validation.is_legal is False
|
||||
assert validation.major_league_count == 27 # 24 + 3
|
||||
assert len(validation.errors) == 1
|
||||
assert "27 players (limit: 26)" in validation.errors[0]
|
||||
assert len(validation.suggestions) == 1
|
||||
assert "Drop 1 ML player" in validation.suggestions[0]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validate_transaction_empty(self, builder, mock_roster):
|
||||
async def test_validate_transaction_empty(self, builder):
|
||||
"""Test validation of empty transaction."""
|
||||
with patch.object(builder, '_current_roster', mock_roster):
|
||||
with patch.object(builder, '_roster_loaded', True):
|
||||
validation = await builder.validate_transaction()
|
||||
validation = await builder.validate_transaction()
|
||||
|
||||
assert validation.is_legal is True # Empty transaction is legal
|
||||
assert validation.major_league_count == 24 # No changes
|
||||
assert len(validation.suggestions) == 1
|
||||
assert "Add player moves" in validation.suggestions[0]
|
||||
assert validation.is_legal is True # Empty transaction is legal
|
||||
assert validation.major_league_count == 24 # No changes
|
||||
assert len(validation.suggestions) == 1
|
||||
assert "Add player moves" in validation.suggestions[0]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validate_transaction_over_swar_cap(self, mock_roster):
|
||||
@ -416,39 +432,43 @@ class TestTransactionBuilder:
|
||||
# Create a team with a low salary cap (30.0)
|
||||
low_cap_team = Team(
|
||||
id=499,
|
||||
abbrev='WV',
|
||||
sname='Black Bears',
|
||||
lname='West Virginia Black Bears',
|
||||
abbrev="WV",
|
||||
sname="Black Bears",
|
||||
lname="West Virginia Black Bears",
|
||||
season=12,
|
||||
salary_cap=30.0 # Low cap - roster has 24 * 1.5 = 36.0 sWAR
|
||||
salary_cap=30.0, # Low cap - roster has 24 * 1.5 = 36.0 sWAR
|
||||
)
|
||||
builder = TransactionBuilder(low_cap_team, user_id=12345)
|
||||
mock_svc = AsyncMock()
|
||||
mock_svc.get_current_roster.return_value = mock_roster
|
||||
builder = TransactionBuilder(low_cap_team, user_id=12345, roster_svc=mock_svc)
|
||||
|
||||
with patch.object(builder, '_current_roster', mock_roster):
|
||||
with patch.object(builder, '_roster_loaded', True):
|
||||
validation = await builder.validate_transaction()
|
||||
validation = await builder.validate_transaction()
|
||||
|
||||
# Should fail due to sWAR cap (36.0 > 30.0)
|
||||
assert validation.is_legal is False
|
||||
assert validation.major_league_swar == 36.0 # 24 players * 1.5 wara
|
||||
assert validation.major_league_swar_cap == 30.0
|
||||
assert any("sWAR would be 36.00 (cap: 30.0)" in error for error in validation.errors)
|
||||
assert any("Remove 6.00 sWAR" in suggestion for suggestion in validation.suggestions)
|
||||
# Should fail due to sWAR cap (36.0 > 30.0)
|
||||
assert validation.is_legal is False
|
||||
assert validation.major_league_swar == 36.0 # 24 players * 1.5 wara
|
||||
assert validation.major_league_swar_cap == 30.0
|
||||
assert any(
|
||||
"sWAR would be 36.00 (cap: 30.0)" in error for error in validation.errors
|
||||
)
|
||||
assert any(
|
||||
"Remove 6.00 sWAR" in suggestion for suggestion in validation.suggestions
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validate_transaction_under_swar_cap(self, mock_team, mock_roster):
|
||||
"""Test validation passes when sWAR is under team cap."""
|
||||
# mock_team has salary_cap=50.0, roster has 36.0 sWAR
|
||||
builder = TransactionBuilder(mock_team, user_id=12345)
|
||||
mock_svc = AsyncMock()
|
||||
mock_svc.get_current_roster.return_value = mock_roster
|
||||
builder = TransactionBuilder(mock_team, user_id=12345, roster_svc=mock_svc)
|
||||
|
||||
with patch.object(builder, '_current_roster', mock_roster):
|
||||
with patch.object(builder, '_roster_loaded', True):
|
||||
validation = await builder.validate_transaction()
|
||||
validation = await builder.validate_transaction()
|
||||
|
||||
# Should pass - 36.0 < 50.0
|
||||
assert validation.is_legal is True
|
||||
assert validation.major_league_swar == 36.0
|
||||
assert validation.major_league_swar_cap == 50.0
|
||||
# Should pass - 36.0 < 50.0
|
||||
assert validation.is_legal is True
|
||||
assert validation.major_league_swar == 36.0
|
||||
assert validation.major_league_swar_cap == 50.0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_swar_status_display_over_cap(self):
|
||||
@ -462,7 +482,7 @@ class TestTransactionBuilder:
|
||||
suggestions=[],
|
||||
major_league_swar=35.5,
|
||||
minor_league_swar=3.0,
|
||||
major_league_swar_cap=33.5
|
||||
major_league_swar_cap=33.5,
|
||||
)
|
||||
assert "❌" in validation.major_league_swar_status
|
||||
assert "35.50/33.5" in validation.major_league_swar_status
|
||||
@ -480,7 +500,7 @@ class TestTransactionBuilder:
|
||||
suggestions=[],
|
||||
major_league_swar=31.0,
|
||||
minor_league_swar=3.0,
|
||||
major_league_swar_cap=33.5
|
||||
major_league_swar_cap=33.5,
|
||||
)
|
||||
assert "✅" in validation.major_league_swar_status
|
||||
assert "31.00/33.5" in validation.major_league_swar_status
|
||||
@ -493,89 +513,87 @@ class TestTransactionBuilder:
|
||||
await builder.submit_transaction(week=11)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_submit_transaction_illegal(self, builder, mock_roster):
|
||||
async def test_submit_transaction_illegal(self, builder):
|
||||
"""Test submitting illegal transaction fails."""
|
||||
with patch.object(builder, '_current_roster', mock_roster):
|
||||
with patch.object(builder, '_roster_loaded', True):
|
||||
# Add moves that exceed limit
|
||||
for i in range(3): # 24 + 3 = 27 > 25
|
||||
player = Player(
|
||||
id=4000 + i,
|
||||
name=f'Illegal Player {i}',
|
||||
wara=1.5,
|
||||
season=12,
|
||||
pos_1='OF'
|
||||
)
|
||||
move = TransactionMove(
|
||||
player=player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
)
|
||||
success, _ = await builder.add_move(move, check_pending_transactions=False)
|
||||
assert success
|
||||
# Add moves that exceed limit
|
||||
for i in range(3): # 24 + 3 = 27 > 25
|
||||
player = Player(
|
||||
id=4000 + i,
|
||||
name=f"Illegal Player {i}",
|
||||
wara=1.5,
|
||||
season=12,
|
||||
pos_1="OF",
|
||||
)
|
||||
move = TransactionMove(
|
||||
player=player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team,
|
||||
)
|
||||
success, _ = await builder.add_move(move, check_pending_transactions=False)
|
||||
assert success
|
||||
|
||||
with pytest.raises(ValueError, match="Cannot submit illegal transaction"):
|
||||
await builder.submit_transaction(week=11)
|
||||
with pytest.raises(ValueError, match="Cannot submit illegal transaction"):
|
||||
await builder.submit_transaction(week=11)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_submit_transaction_success(self, builder, mock_roster, mock_player):
|
||||
async def test_submit_transaction_success(self, builder, mock_player):
|
||||
"""Test successful transaction submission."""
|
||||
with patch.object(builder, '_current_roster', mock_roster):
|
||||
with patch.object(builder, '_roster_loaded', True):
|
||||
# Add a legal move
|
||||
move = TransactionMove(
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
)
|
||||
success, _ = await builder.add_move(move, check_pending_transactions=False)
|
||||
assert success
|
||||
# Add a legal move
|
||||
move = TransactionMove(
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team,
|
||||
)
|
||||
success, _ = await builder.add_move(move, check_pending_transactions=False)
|
||||
assert success
|
||||
|
||||
transactions = await builder.submit_transaction(week=11)
|
||||
transactions = await builder.submit_transaction(week=11)
|
||||
|
||||
assert len(transactions) == 1
|
||||
transaction = transactions[0]
|
||||
assert isinstance(transaction, Transaction)
|
||||
assert transaction.week == 11
|
||||
assert transaction.season == 12
|
||||
assert transaction.player == mock_player
|
||||
assert transaction.newteam == builder.team
|
||||
assert "Season-012-Week-11-" in transaction.moveid
|
||||
assert len(transactions) == 1
|
||||
transaction = transactions[0]
|
||||
assert isinstance(transaction, Transaction)
|
||||
assert transaction.week == 11
|
||||
assert transaction.season == 12
|
||||
assert transaction.player == mock_player
|
||||
assert transaction.newteam == builder.team
|
||||
assert "Season-012-Week-11-" in transaction.moveid
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_submit_complex_transaction(self, builder, mock_roster):
|
||||
async def test_submit_complex_transaction(self, builder):
|
||||
"""Test submitting transaction with multiple moves."""
|
||||
with patch.object(builder, '_current_roster', mock_roster):
|
||||
with patch.object(builder, '_roster_loaded', True):
|
||||
# Add one player and drop one player (net zero)
|
||||
add_player = Player(id=5001, name='Add Player', wara=2.0, season=12, pos_1='OF')
|
||||
drop_player = Player(id=5002, name='Drop Player', wara=1.0, season=12, pos_1='OF')
|
||||
# Add one player and drop one player (net zero)
|
||||
add_player = Player(id=5001, name="Add Player", wara=2.0, season=12, pos_1="OF")
|
||||
drop_player = Player(
|
||||
id=5002, name="Drop Player", wara=1.0, season=12, pos_1="OF"
|
||||
)
|
||||
|
||||
add_move = TransactionMove(
|
||||
player=add_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
)
|
||||
add_move = TransactionMove(
|
||||
player=add_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team,
|
||||
)
|
||||
|
||||
drop_move = TransactionMove(
|
||||
player=drop_player,
|
||||
from_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_roster=RosterType.FREE_AGENCY,
|
||||
from_team=builder.team
|
||||
)
|
||||
drop_move = TransactionMove(
|
||||
player=drop_player,
|
||||
from_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_roster=RosterType.FREE_AGENCY,
|
||||
from_team=builder.team,
|
||||
)
|
||||
|
||||
success1, _ = await builder.add_move(add_move, check_pending_transactions=False)
|
||||
success2, _ = await builder.add_move(drop_move, check_pending_transactions=False)
|
||||
assert success1 and success2
|
||||
success1, _ = await builder.add_move(add_move, check_pending_transactions=False)
|
||||
success2, _ = await builder.add_move(
|
||||
drop_move, check_pending_transactions=False
|
||||
)
|
||||
assert success1 and success2
|
||||
|
||||
transactions = await builder.submit_transaction(week=11)
|
||||
transactions = await builder.submit_transaction(week=11)
|
||||
|
||||
assert len(transactions) == 2
|
||||
# Both transactions should have the same move_id
|
||||
assert transactions[0].moveid == transactions[1].moveid
|
||||
assert len(transactions) == 2
|
||||
# Both transactions should have the same move_id
|
||||
assert transactions[0].moveid == transactions[1].moveid
|
||||
|
||||
|
||||
class TestTransactionMove:
|
||||
@ -584,12 +602,18 @@ class TestTransactionMove:
|
||||
@pytest.fixture
|
||||
def mock_player(self):
|
||||
"""Create a mock player."""
|
||||
return Player(id=123, name='Test Player', wara=2.0, season=12, pos_1='OF')
|
||||
return Player(id=123, name="Test Player", wara=2.0, season=12, pos_1="OF")
|
||||
|
||||
@pytest.fixture
|
||||
def mock_team(self):
|
||||
"""Create a mock team."""
|
||||
return Team(id=499, abbrev='WV', sname='Black Bears', lname='West Virginia Black Bears', season=12)
|
||||
return Team(
|
||||
id=499,
|
||||
abbrev="WV",
|
||||
sname="Black Bears",
|
||||
lname="West Virginia Black Bears",
|
||||
season=12,
|
||||
)
|
||||
|
||||
def test_add_move_description(self, mock_player, mock_team):
|
||||
"""Test ADD move description."""
|
||||
@ -597,7 +621,7 @@ class TestTransactionMove:
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=mock_team
|
||||
to_team=mock_team,
|
||||
)
|
||||
|
||||
expected = "➕ Test Player: FA → WV (ML)"
|
||||
@ -609,7 +633,7 @@ class TestTransactionMove:
|
||||
player=mock_player,
|
||||
from_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_roster=RosterType.FREE_AGENCY,
|
||||
from_team=mock_team
|
||||
from_team=mock_team,
|
||||
)
|
||||
|
||||
expected = "➖ Test Player: WV (ML) → FA"
|
||||
@ -622,7 +646,7 @@ class TestTransactionMove:
|
||||
from_roster=RosterType.MINOR_LEAGUE,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
from_team=mock_team,
|
||||
to_team=mock_team
|
||||
to_team=mock_team,
|
||||
)
|
||||
|
||||
expected = "⬆️ Test Player: WV (MiL) → WV (ML)"
|
||||
@ -635,7 +659,7 @@ class TestTransactionMove:
|
||||
from_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_roster=RosterType.MINOR_LEAGUE,
|
||||
from_team=mock_team,
|
||||
to_team=mock_team
|
||||
to_team=mock_team,
|
||||
)
|
||||
|
||||
expected = "⬇️ Test Player: WV (ML) → WV (MiL)"
|
||||
@ -653,7 +677,7 @@ class TestRosterValidationResult:
|
||||
minor_league_count=6,
|
||||
warnings=[],
|
||||
errors=[],
|
||||
suggestions=[]
|
||||
suggestions=[],
|
||||
)
|
||||
|
||||
expected = "❌ Major League: 27/26 (Over limit!)"
|
||||
@ -667,7 +691,7 @@ class TestRosterValidationResult:
|
||||
minor_league_count=6,
|
||||
warnings=[],
|
||||
errors=[],
|
||||
suggestions=[]
|
||||
suggestions=[],
|
||||
)
|
||||
|
||||
expected = "✅ Major League: 26/26 (Legal)"
|
||||
@ -681,7 +705,7 @@ class TestRosterValidationResult:
|
||||
minor_league_count=6,
|
||||
warnings=[],
|
||||
errors=[],
|
||||
suggestions=[]
|
||||
suggestions=[],
|
||||
)
|
||||
|
||||
expected = "✅ Major League: 23/26 (Legal)"
|
||||
@ -695,7 +719,7 @@ class TestRosterValidationResult:
|
||||
minor_league_count=7,
|
||||
warnings=[],
|
||||
errors=[],
|
||||
suggestions=[]
|
||||
suggestions=[],
|
||||
)
|
||||
|
||||
expected = "❌ Minor League: 7/6 (Over limit!)"
|
||||
@ -707,7 +731,13 @@ class TestTransactionBuilderGlobalFunctions:
|
||||
|
||||
def test_get_transaction_builder_new(self):
|
||||
"""Test getting new transaction builder."""
|
||||
team = Team(id=499, abbrev='WV', sname='Black Bears', lname='West Virginia Black Bears', season=12)
|
||||
team = Team(
|
||||
id=499,
|
||||
abbrev="WV",
|
||||
sname="Black Bears",
|
||||
lname="West Virginia Black Bears",
|
||||
season=12,
|
||||
)
|
||||
|
||||
builder = get_transaction_builder(user_id=123, team=team)
|
||||
|
||||
@ -717,7 +747,13 @@ class TestTransactionBuilderGlobalFunctions:
|
||||
|
||||
def test_get_transaction_builder_existing(self):
|
||||
"""Test getting existing transaction builder."""
|
||||
team = Team(id=499, abbrev='WV', sname='Black Bears', lname='West Virginia Black Bears', season=12)
|
||||
team = Team(
|
||||
id=499,
|
||||
abbrev="WV",
|
||||
sname="Black Bears",
|
||||
lname="West Virginia Black Bears",
|
||||
season=12,
|
||||
)
|
||||
|
||||
builder1 = get_transaction_builder(user_id=123, team=team)
|
||||
builder2 = get_transaction_builder(user_id=123, team=team)
|
||||
@ -726,7 +762,13 @@ class TestTransactionBuilderGlobalFunctions:
|
||||
|
||||
def test_clear_transaction_builder(self):
|
||||
"""Test clearing transaction builder."""
|
||||
team = Team(id=499, abbrev='WV', sname='Black Bears', lname='West Virginia Black Bears', season=12)
|
||||
team = Team(
|
||||
id=499,
|
||||
abbrev="WV",
|
||||
sname="Black Bears",
|
||||
lname="West Virginia Black Bears",
|
||||
season=12,
|
||||
)
|
||||
|
||||
builder = get_transaction_builder(user_id=123, team=team)
|
||||
assert builder is not None
|
||||
@ -756,22 +798,16 @@ class TestPendingTransactionValidation:
|
||||
"""Create a mock team for testing."""
|
||||
return Team(
|
||||
id=499,
|
||||
abbrev='WV',
|
||||
sname='Black Bears',
|
||||
lname='West Virginia Black Bears',
|
||||
season=12
|
||||
abbrev="WV",
|
||||
sname="Black Bears",
|
||||
lname="West Virginia Black Bears",
|
||||
season=12,
|
||||
)
|
||||
|
||||
@pytest.fixture
|
||||
def mock_player(self):
|
||||
"""Create a mock player for testing."""
|
||||
return Player(
|
||||
id=12472,
|
||||
name='Test Player',
|
||||
wara=2.5,
|
||||
season=12,
|
||||
pos_1='OF'
|
||||
)
|
||||
return Player(id=12472, name="Test Player", wara=2.5, season=12, pos_1="OF")
|
||||
|
||||
@pytest.fixture
|
||||
def builder(self, mock_team):
|
||||
@ -779,7 +815,9 @@ class TestPendingTransactionValidation:
|
||||
return TransactionBuilder(mock_team, user_id=123, season=12)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_move_player_in_pending_transaction_fails(self, builder, mock_player):
|
||||
async def test_add_move_player_in_pending_transaction_fails(
|
||||
self, builder, mock_player
|
||||
):
|
||||
"""
|
||||
Test that adding a player who is already in a pending transaction fails.
|
||||
|
||||
@ -790,11 +828,15 @@ class TestPendingTransactionValidation:
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
to_team=builder.team,
|
||||
)
|
||||
|
||||
with patch('services.transaction_builder.transaction_service') as mock_tx_service:
|
||||
with patch('services.transaction_builder.league_service') as mock_league_service:
|
||||
with patch(
|
||||
"services.transaction_builder.transaction_service"
|
||||
) as mock_tx_service:
|
||||
with patch(
|
||||
"services.transaction_builder.league_service"
|
||||
) as mock_league_service:
|
||||
# Mock that player IS in a pending transaction (claimed by LAA)
|
||||
mock_tx_service.is_player_in_pending_transaction = AsyncMock(
|
||||
return_value=(True, "LAA")
|
||||
@ -813,7 +855,9 @@ class TestPendingTransactionValidation:
|
||||
assert builder.move_count == 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_move_player_not_in_pending_transaction_succeeds(self, builder, mock_player):
|
||||
async def test_add_move_player_not_in_pending_transaction_succeeds(
|
||||
self, builder, mock_player
|
||||
):
|
||||
"""
|
||||
Test that adding a player who is NOT in a pending transaction succeeds.
|
||||
|
||||
@ -824,11 +868,15 @@ class TestPendingTransactionValidation:
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
to_team=builder.team,
|
||||
)
|
||||
|
||||
with patch('services.transaction_builder.transaction_service') as mock_tx_service:
|
||||
with patch('services.transaction_builder.league_service') as mock_league_service:
|
||||
with patch(
|
||||
"services.transaction_builder.transaction_service"
|
||||
) as mock_tx_service:
|
||||
with patch(
|
||||
"services.transaction_builder.league_service"
|
||||
) as mock_league_service:
|
||||
# Mock that player is NOT in a pending transaction
|
||||
mock_tx_service.is_player_in_pending_transaction = AsyncMock(
|
||||
return_value=(False, None)
|
||||
@ -856,11 +904,13 @@ class TestPendingTransactionValidation:
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
to_team=builder.team,
|
||||
)
|
||||
|
||||
# Even if the service would return True, the check should be skipped
|
||||
with patch('services.transaction_builder.transaction_service') as mock_tx_service:
|
||||
with patch(
|
||||
"services.transaction_builder.transaction_service"
|
||||
) as mock_tx_service:
|
||||
# This mock should NOT be called when check_pending_transactions=False
|
||||
mock_tx_service.is_player_in_pending_transaction = AsyncMock(
|
||||
return_value=(True, "LAA")
|
||||
@ -888,10 +938,12 @@ class TestPendingTransactionValidation:
|
||||
player=mock_player,
|
||||
from_roster=RosterType.FREE_AGENCY,
|
||||
to_roster=RosterType.MAJOR_LEAGUE,
|
||||
to_team=builder.team
|
||||
to_team=builder.team,
|
||||
)
|
||||
|
||||
with patch('services.transaction_builder.transaction_service') as mock_tx_service:
|
||||
with patch(
|
||||
"services.transaction_builder.transaction_service"
|
||||
) as mock_tx_service:
|
||||
mock_tx_service.is_player_in_pending_transaction = AsyncMock(
|
||||
return_value=(False, None)
|
||||
)
|
||||
@ -902,4 +954,4 @@ class TestPendingTransactionValidation:
|
||||
# Verify the check was called with the explicit week
|
||||
mock_tx_service.is_player_in_pending_transaction.assert_called_once()
|
||||
call_args = mock_tx_service.is_player_in_pending_transaction.call_args
|
||||
assert call_args.kwargs['week'] == 15
|
||||
assert call_args.kwargs["week"] == 15
|
||||
|
||||
Loading…
Reference in New Issue
Block a user