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>
278 lines
11 KiB
Python
278 lines
11 KiB
Python
"""
|
|
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"
|