major-domo-v2/tests/test_roster_validation_org_affiliates.py
Cal Corum aa27769ed6 fix: roster validation now includes org affiliate transactions (closes #49)
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>
2026-03-01 10:45:21 -06:00

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"