diff --git a/commands/draft/admin.py b/commands/draft/admin.py index bb87170..011765c 100644 --- a/commands/draft/admin.py +++ b/commands/draft/admin.py @@ -173,6 +173,13 @@ class DraftAdminGroup(app_commands.Group): if pick.owner: description += f"\n\n{pick.owner.abbrev} {pick.owner.sname} is now on the clock." + # Add timer status + if updated.timer and updated.pick_deadline: + deadline_timestamp = int(updated.pick_deadline.timestamp()) + description += f"\n\n⏱️ **Timer Active** - Deadline " + else: + description += "\n\n⏸️ **Timer Inactive**" + embed = EmbedTemplate.success("Pick Updated", description) await interaction.followup.send(embed=embed) diff --git a/commands/draft/picks.py b/commands/draft/picks.py index 837d272..e0366db 100644 --- a/commands/draft/picks.py +++ b/commands/draft/picks.py @@ -215,12 +215,12 @@ class DraftPicksCog(commands.Cog): await interaction.followup.send(embed=embed) return - is_valid, projected_total = await validate_cap_space(roster, player_obj.wara) + is_valid, projected_total, cap_limit = await validate_cap_space(roster, player_obj.wara, team) if not is_valid: embed = await create_pick_illegal_embed( "Cap Space Exceeded", - f"Drafting {player_obj.name} would put you at {projected_total:.2f} sWAR (limit: {config.swar_cap_limit:.2f})." + f"Drafting {player_obj.name} would put you at {projected_total:.2f} sWAR (limit: {cap_limit:.2f})." ) await interaction.followup.send(embed=embed) return @@ -253,7 +253,8 @@ class DraftPicksCog(commands.Cog): player_obj, team, current_pick.overall, - projected_total + projected_total, + cap_limit ) await interaction.followup.send(embed=success_embed) diff --git a/models/draft_pick.py b/models/draft_pick.py index 3a39dc5..948fb3a 100644 --- a/models/draft_pick.py +++ b/models/draft_pick.py @@ -2,9 +2,15 @@ Draft pick model Represents individual draft picks with team and player relationships. + +API FIELD MAPPING: +The API returns fields without _id suffix (origowner, owner, player). +When the API short_output=false, these fields contain full Team/Player objects. +When short_output=true (or default), they contain integer IDs. +We use Pydantic aliases to handle both cases. """ -from typing import Optional -from pydantic import Field +from typing import Optional, Any, Dict, Union +from pydantic import Field, field_validator, model_validator from models.base import SBABaseModel from models.team import Team @@ -13,21 +19,79 @@ from models.player import Player class DraftPick(SBABaseModel): """Draft pick model representing a single draft selection.""" - + season: int = Field(..., description="Draft season") overall: int = Field(..., description="Overall pick number") round: int = Field(..., description="Draft round") - - # Team relationships - IDs are required, objects are optional + + # Team relationships - IDs extracted from API response + # API returns "origowner" which can be int or Team object origowner_id: int = Field(..., description="Original owning team ID") origowner: Optional[Team] = Field(None, description="Original owning team (populated when needed)") - + + # API returns "owner" which can be int or Team object owner_id: Optional[int] = Field(None, description="Current owning team ID") owner: Optional[Team] = Field(None, description="Current owning team (populated when needed)") - - # Player selection + + # Player selection - API returns "player" which can be int or Player object player_id: Optional[int] = Field(None, description="Selected player ID") player: Optional[Player] = Field(None, description="Selected player (populated when needed)") + + @classmethod + def from_api_data(cls, data: Dict[str, Any]) -> 'DraftPick': + """ + Create DraftPick from API response data. + + Handles API field mapping: + - API returns 'origowner', 'owner', 'player' (without _id suffix) + - These can be integer IDs or full objects depending on short_output setting + """ + if not data: + raise ValueError("Cannot create DraftPick from empty data") + + # Make a copy to avoid modifying the original + parsed = dict(data) + + # Handle origowner: can be int ID or Team object + if 'origowner' in parsed: + origowner = parsed.pop('origowner') + if isinstance(origowner, dict): + # Full Team object from API + parsed['origowner'] = Team.from_api_data(origowner) + parsed['origowner_id'] = origowner.get('id', origowner) + elif isinstance(origowner, int): + # Just the ID + parsed['origowner_id'] = origowner + elif origowner is not None: + parsed['origowner_id'] = int(origowner) + + # Handle owner: can be int ID or Team object + if 'owner' in parsed: + owner = parsed.pop('owner') + if isinstance(owner, dict): + # Full Team object from API + parsed['owner'] = Team.from_api_data(owner) + parsed['owner_id'] = owner.get('id', owner) + elif isinstance(owner, int): + # Just the ID + parsed['owner_id'] = owner + elif owner is not None: + parsed['owner_id'] = int(owner) + + # Handle player: can be int ID or Player object (or None) + if 'player' in parsed: + player = parsed.pop('player') + if isinstance(player, dict): + # Full Player object from API + parsed['player'] = Player.from_api_data(player) + parsed['player_id'] = player.get('id', player) + elif isinstance(player, int): + # Just the ID + parsed['player_id'] = player + elif player is not None: + parsed['player_id'] = int(player) + + return cls(**parsed) @property def is_traded(self) -> bool: diff --git a/models/team.py b/models/team.py index 10f9ace..a21d365 100644 --- a/models/team.py +++ b/models/team.py @@ -47,6 +47,7 @@ class Team(SBABaseModel): thumbnail: Optional[str] = Field(None, description="Team thumbnail URL") color: Optional[str] = Field(None, description="Primary team color") dice_color: Optional[str] = Field(None, description="Dice rolling color") + salary_cap: Optional[float] = Field(None, description="Team-specific salary cap (None uses default)") @classmethod def from_api_data(cls, data: dict) -> 'Team': diff --git a/services/draft_pick_service.py b/services/draft_pick_service.py index abbd664..a081998 100644 --- a/services/draft_pick_service.py +++ b/services/draft_pick_service.py @@ -33,6 +33,33 @@ class DraftPickService(BaseService[DraftPick]): super().__init__(DraftPick, 'draftpicks') logger.debug("DraftPickService initialized") + def _extract_items_and_count_from_response(self, data): + """ + Override to handle API quirk: GET returns 'picks' instead of 'draftpicks'. + + Args: + data: API response data + + Returns: + Tuple of (items list, total count) + """ + if isinstance(data, list): + return data, len(data) + + if not isinstance(data, dict): + logger.warning(f"Unexpected response format: {type(data)}") + return [], 0 + + # Get count + count = data.get('count', 0) + + # API returns items under 'picks' key (not 'draftpicks') + if 'picks' in data and isinstance(data['picks'], list): + return data['picks'], count or len(data['picks']) + + # Fallback to standard extraction + return super()._extract_items_and_count_from_response(data) + async def get_pick(self, season: int, overall: int) -> Optional[DraftPick]: """ Get specific pick by season and overall number. diff --git a/tests/test_utils_draft_helpers.py b/tests/test_utils_draft_helpers.py new file mode 100644 index 0000000..4217509 --- /dev/null +++ b/tests/test_utils_draft_helpers.py @@ -0,0 +1,534 @@ +""" +Unit tests for draft helper functions in utils/draft_helpers.py. + +These tests verify: +1. calculate_pick_details() correctly handles linear and snake draft formats +2. calculate_overall_from_round_position() is the inverse of calculate_pick_details() +3. validate_cap_space() correctly validates roster cap space with team-specific caps +4. Other helper functions work correctly + +Why these tests matter: +- Draft pick calculations are critical for correct draft order +- Cap space validation prevents illegal roster configurations +- These functions are used throughout the draft system +""" + +import pytest +from utils.draft_helpers import ( + calculate_pick_details, + calculate_overall_from_round_position, + validate_cap_space, + format_pick_display, + get_next_pick_overall, + is_draft_complete, + get_round_name, +) + + +class TestCalculatePickDetails: + """Tests for calculate_pick_details() function.""" + + def test_round_1_pick_1(self): + """ + Overall pick 1 should be Round 1, Pick 1. + + Why: First pick of the draft is the simplest case. + """ + round_num, position = calculate_pick_details(1) + assert round_num == 1 + assert position == 1 + + def test_round_1_pick_16(self): + """ + Overall pick 16 should be Round 1, Pick 16. + + Why: Last pick of round 1 in a 16-team draft. + """ + round_num, position = calculate_pick_details(16) + assert round_num == 1 + assert position == 16 + + def test_round_2_pick_1(self): + """ + Overall pick 17 should be Round 2, Pick 1. + + Why: First pick of round 2 (linear format for rounds 1-10). + """ + round_num, position = calculate_pick_details(17) + assert round_num == 2 + assert position == 1 + + def test_round_10_pick_16(self): + """ + Overall pick 160 should be Round 10, Pick 16. + + Why: Last pick of linear draft section. + """ + round_num, position = calculate_pick_details(160) + assert round_num == 10 + assert position == 16 + + def test_round_11_pick_1_snake_begins(self): + """ + Overall pick 161 should be Round 11, Pick 1. + + Why: First pick of snake draft. Same team as Round 10 Pick 16 + gets first pick of Round 11. + """ + round_num, position = calculate_pick_details(161) + assert round_num == 11 + assert position == 1 + + def test_round_11_pick_16(self): + """ + Overall pick 176 should be Round 11, Pick 16. + + Why: Last pick of round 11 (odd snake round = forward order). + """ + round_num, position = calculate_pick_details(176) + assert round_num == 11 + assert position == 16 + + def test_round_12_snake_reverses(self): + """ + Round 12 should be in reverse order (snake). + + Why: Even rounds in snake draft reverse the order. + """ + # Pick 177 = Round 12, Pick 16 (starts with last team) + round_num, position = calculate_pick_details(177) + assert round_num == 12 + assert position == 16 + + # Pick 192 = Round 12, Pick 1 (ends with first team) + round_num, position = calculate_pick_details(192) + assert round_num == 12 + assert position == 1 + + +class TestCalculateOverallFromRoundPosition: + """Tests for calculate_overall_from_round_position() function.""" + + def test_round_1_pick_1(self): + """Round 1, Pick 1 should be overall pick 1.""" + overall = calculate_overall_from_round_position(1, 1) + assert overall == 1 + + def test_round_1_pick_16(self): + """Round 1, Pick 16 should be overall pick 16.""" + overall = calculate_overall_from_round_position(1, 16) + assert overall == 16 + + def test_round_10_pick_16(self): + """Round 10, Pick 16 should be overall pick 160.""" + overall = calculate_overall_from_round_position(10, 16) + assert overall == 160 + + def test_round_11_pick_1(self): + """Round 11, Pick 1 should be overall pick 161.""" + overall = calculate_overall_from_round_position(11, 1) + assert overall == 161 + + def test_round_12_pick_16_snake(self): + """Round 12, Pick 16 should be overall pick 177 (snake reverses).""" + overall = calculate_overall_from_round_position(12, 16) + assert overall == 177 + + def test_inverse_relationship_linear(self): + """ + calculate_overall_from_round_position should be inverse of calculate_pick_details + for linear rounds (1-10). + + Why: These functions must be inverses for draft logic to work correctly. + """ + for overall in range(1, 161): # All linear picks + round_num, position = calculate_pick_details(overall) + calculated_overall = calculate_overall_from_round_position(round_num, position) + assert calculated_overall == overall, f"Failed for overall={overall}" + + def test_inverse_relationship_snake(self): + """ + calculate_overall_from_round_position should be inverse of calculate_pick_details + for snake rounds (11+). + + Why: These functions must be inverses for draft logic to work correctly. + """ + for overall in range(161, 257): # First 6 snake rounds + round_num, position = calculate_pick_details(overall) + calculated_overall = calculate_overall_from_round_position(round_num, position) + assert calculated_overall == overall, f"Failed for overall={overall}" + + +class TestValidateCapSpace: + """Tests for validate_cap_space() function.""" + + @pytest.mark.asyncio + async def test_valid_under_cap(self): + """ + Drafting a player that keeps team under cap should be valid. + + Why: Normal case - team is under cap and pick should be allowed. + The 26 cheapest players are summed (all 3 in this case since < 26). + """ + roster = { + 'active': { + 'players': [ + {'id': 1, 'name': 'Player 1', 'wara': 5.0}, + {'id': 2, 'name': 'Player 2', 'wara': 4.0}, + ], + 'WARa': 9.0 + } + } + new_player_wara = 3.0 + + is_valid, projected_total, cap_limit = await validate_cap_space(roster, new_player_wara) + + assert is_valid is True + assert projected_total == 12.0 # 3 + 4 + 5 (all players, sorted ascending) + assert cap_limit == 32.0 # Default cap + + @pytest.mark.asyncio + async def test_invalid_over_cap(self): + """ + Drafting a player that puts team over cap should be invalid. + + Why: Must prevent illegal roster configurations. + With 26 players all at 1.5 WAR, sum = 39.0 which exceeds 32.0 cap. + """ + # Create roster with 25 players at 1.5 WAR each + players = [{'id': i, 'name': f'Player {i}', 'wara': 1.5} for i in range(25)] + roster = { + 'active': { + 'players': players, + 'WARa': 37.5 # 25 * 1.5 + } + } + new_player_wara = 1.5 # Adding another 1.5 player = 26 * 1.5 = 39.0 + + is_valid, projected_total, cap_limit = await validate_cap_space(roster, new_player_wara) + + assert is_valid is False + assert projected_total == 39.0 # 26 * 1.5 + assert cap_limit == 32.0 + + @pytest.mark.asyncio + async def test_team_specific_cap(self): + """ + Should use team's custom salary cap when provided. + + Why: Some teams have different caps (expansion, penalties, etc.) + """ + roster = { + 'active': { + 'players': [ + {'id': 1, 'name': 'Player 1', 'wara': 10.0}, + {'id': 2, 'name': 'Player 2', 'wara': 10.0}, + ], + 'WARa': 20.0 + } + } + team = {'abbrev': 'EXP', 'salary_cap': 25.0} # Expansion team with lower cap + new_player_wara = 6.0 # Total = 26.0 which exceeds 25.0 cap + + is_valid, projected_total, cap_limit = await validate_cap_space(roster, new_player_wara, team) + + assert is_valid is False # Over custom 25.0 cap + assert projected_total == 26.0 # 6 + 10 + 10 (sorted ascending) + assert cap_limit == 25.0 + + @pytest.mark.asyncio + async def test_team_with_none_cap_uses_default(self): + """ + Team with salary_cap=None should use default cap. + + Why: Backwards compatibility for teams without custom caps. + """ + roster = { + 'active': { + 'players': [ + {'id': 1, 'name': 'Player 1', 'wara': 10.0}, + ], + 'WARa': 10.0 + } + } + team = {'abbrev': 'STD', 'salary_cap': None} + new_player_wara = 5.0 + + is_valid, projected_total, cap_limit = await validate_cap_space(roster, new_player_wara, team) + + assert is_valid is True + assert projected_total == 15.0 # 5 + 10 + assert cap_limit == 32.0 # Default + + @pytest.mark.asyncio + async def test_cap_counting_logic_cheapest_26(self): + """ + Only the 26 CHEAPEST players should count toward cap. + + Why: League rules - expensive stars can be "excluded" if you have + enough cheap depth players. This rewards roster construction. + """ + # Create 27 players: 1 expensive star (10.0) and 26 cheap players (1.0 each) + players = [{'id': 0, 'name': 'Star', 'wara': 10.0}] # Expensive star + for i in range(1, 27): + players.append({'id': i, 'name': f'Cheap {i}', 'wara': 1.0}) + + roster = { + 'active': { + 'players': players, + 'WARa': sum(p['wara'] for p in players) # 10 + 26 = 36 + } + } + new_player_wara = 1.0 # Adding another cheap player + + is_valid, projected_total, cap_limit = await validate_cap_space(roster, new_player_wara) + + # With 28 players total, only cheapest 26 count + # Sorted ascending: 27 players at 1.0, then 1 at 10.0 + # Cheapest 26 = 26 * 1.0 = 26.0 (the star is EXCLUDED) + assert is_valid is True + assert projected_total == 26.0 + assert cap_limit == 32.0 + + @pytest.mark.asyncio + async def test_invalid_roster_structure(self): + """ + Invalid roster structure should raise ValueError. + + Why: Defensive programming - catch malformed data early. + """ + with pytest.raises(ValueError, match="Invalid roster structure"): + await validate_cap_space({}, 1.0) + + with pytest.raises(ValueError, match="Invalid roster structure"): + await validate_cap_space(None, 1.0) + + with pytest.raises(ValueError, match="Invalid roster structure"): + await validate_cap_space({'other': {}}, 1.0) + + @pytest.mark.asyncio + async def test_empty_roster(self): + """ + Empty roster should allow any player (well under cap). + + Why: First pick of draft has empty roster. + """ + roster = { + 'active': { + 'players': [], + 'WARa': 0.0 + } + } + new_player_wara = 5.0 + + is_valid, projected_total, cap_limit = await validate_cap_space(roster, new_player_wara) + + assert is_valid is True + assert projected_total == 5.0 + + @pytest.mark.asyncio + async def test_tolerance_boundary(self): + """ + Values at or just below cap + tolerance should be valid. + + Why: Floating point tolerance prevents false positives. + """ + # Create 25 players at 1.28 WAR each = 32.0 total + players = [{'id': i, 'name': f'Player {i}', 'wara': 1.28} for i in range(25)] + roster = { + 'active': { + 'players': players, + 'WARa': 32.0 + } + } + + # Adding 0.0 WAR player keeps us at exactly cap - should be valid + is_valid, projected_total, _ = await validate_cap_space(roster, 0.0) + assert is_valid is True + assert abs(projected_total - 32.0) < 0.01 + + # Adding 0.002 WAR player puts us just over tolerance - should be invalid + is_valid, _, _ = await validate_cap_space(roster, 0.003) + assert is_valid is False + + @pytest.mark.asyncio + async def test_star_exclusion_scenario(self): + """ + Test realistic scenario where an expensive star is excluded from cap. + + Why: This is the key feature - teams can build around stars by + surrounding them with cheap depth players. + """ + # 26 cheap players at 1.0 WAR each + players = [{'id': i, 'name': f'Depth {i}', 'wara': 1.0} for i in range(26)] + roster = { + 'active': { + 'players': players, + 'WARa': 26.0 + } + } + + # Drafting a 10.0 WAR superstar + # With 27 players, cheapest 26 count = 26 * 1.0 = 26.0 (star excluded!) + is_valid, projected_total, cap_limit = await validate_cap_space(roster, 10.0) + + assert is_valid is True + assert projected_total == 26.0 # Star is excluded from cap calculation + assert cap_limit == 32.0 + + +class TestFormatPickDisplay: + """Tests for format_pick_display() function.""" + + def test_format_pick_1(self): + """First pick should display correctly.""" + result = format_pick_display(1) + assert result == "Round 1, Pick 1 (Overall #1)" + + def test_format_pick_45(self): + """Middle pick should display correctly.""" + result = format_pick_display(45) + assert "Round 3" in result + assert "Overall #45" in result + + def test_format_pick_161(self): + """First snake pick should display correctly.""" + result = format_pick_display(161) + assert "Round 11" in result + assert "Overall #161" in result + + +class TestGetNextPickOverall: + """Tests for get_next_pick_overall() function.""" + + def test_next_pick(self): + """Next pick should increment by 1.""" + assert get_next_pick_overall(1) == 2 + assert get_next_pick_overall(160) == 161 + assert get_next_pick_overall(512) == 513 + + +class TestIsDraftComplete: + """Tests for is_draft_complete() function.""" + + def test_draft_not_complete(self): + """Draft should not be complete before total picks.""" + assert is_draft_complete(1, total_picks=512) is False + assert is_draft_complete(511, total_picks=512) is False + assert is_draft_complete(512, total_picks=512) is False + + def test_draft_complete(self): + """Draft should be complete after total picks.""" + assert is_draft_complete(513, total_picks=512) is True + assert is_draft_complete(600, total_picks=512) is True + + +class TestGetRoundName: + """Tests for get_round_name() function.""" + + def test_round_1(self): + """Round 1 should just say 'Round 1'.""" + assert get_round_name(1) == "Round 1" + + def test_round_11_snake_begins(self): + """Round 11 should indicate snake draft begins.""" + result = get_round_name(11) + assert "Round 11" in result + assert "Snake" in result + + def test_regular_round(self): + """Regular rounds should just show round number.""" + assert get_round_name(5) == "Round 5" + assert get_round_name(20) == "Round 20" + + +class TestRealTeamModelIntegration: + """Integration tests using the actual Team Pydantic model.""" + + @pytest.mark.asyncio + async def test_validate_cap_space_with_real_team_model(self): + """ + validate_cap_space should work with real Team Pydantic model. + + Why: End-to-end test with actual production model. + """ + from models.team import Team + + roster = { + 'active': { + 'players': [ + {'id': 1, 'name': 'Star', 'wara': 8.0}, + {'id': 2, 'name': 'Good', 'wara': 4.0}, + ], + 'WARa': 12.0 + } + } + + # Team with custom cap of 20.0 + team = Team( + id=1, + abbrev='EXP', + sname='Expansion', + lname='Expansion Team', + season=12, + salary_cap=20.0 + ) + + # Adding 10.0 WAR player: sorted ascending [4.0, 8.0, 10.0] = 22.0 total + # 22.0 > 20.0 cap, so invalid + is_valid, projected_total, cap_limit = await validate_cap_space(roster, 10.0, team) + + assert is_valid is False + assert projected_total == 22.0 # 4 + 8 + 10 + assert cap_limit == 20.0 + + # Adding 5.0 WAR player: sorted ascending [4.0, 5.0, 8.0] = 17.0 total + # 17.0 < 20.0 cap, so valid + is_valid, projected_total, cap_limit = await validate_cap_space(roster, 5.0, team) + + assert is_valid is True + assert projected_total == 17.0 # 4 + 5 + 8 + assert cap_limit == 20.0 + + @pytest.mark.asyncio + async def test_realistic_draft_scenario(self): + """ + Test a realistic draft scenario where team has built around stars. + + Why: Validates the complete workflow with real Team model and + demonstrates the cap exclusion mechanic working as intended. + """ + from models.team import Team + + # Team has 2 superstars (8.0, 7.0) and 25 cheap depth players (1.0 each) + players = [ + {'id': 0, 'name': 'Superstar 1', 'wara': 8.0}, + {'id': 1, 'name': 'Superstar 2', 'wara': 7.0}, + ] + for i in range(2, 27): + players.append({'id': i, 'name': f'Depth {i}', 'wara': 1.0}) + + roster = { + 'active': { + 'players': players, + 'WARa': sum(p['wara'] for p in players) # 8 + 7 + 25 = 40.0 + } + } + + team = Team( + id=1, + abbrev='STR', + sname='Stars', + lname='All-Stars Team', + season=12, + salary_cap=None # Use default 32.0 + ) + + # Draft another 1.0 WAR depth player + # With 28 players, only cheapest 26 count + # Sorted: [1.0 x 26, 7.0, 8.0] - cheapest 26 = 26 * 1.0 = 26.0 + is_valid, projected_total, cap_limit = await validate_cap_space(roster, 1.0, team) + + assert is_valid is True + assert projected_total == 26.0 # Both superstars excluded! + assert cap_limit == 32.0 diff --git a/tests/test_utils_helpers.py b/tests/test_utils_helpers.py new file mode 100644 index 0000000..f667e14 --- /dev/null +++ b/tests/test_utils_helpers.py @@ -0,0 +1,421 @@ +""" +Unit tests for salary cap helper functions in utils/helpers.py. + +These tests verify: +1. get_team_salary_cap() returns correct cap values with fallback behavior +2. exceeds_salary_cap() correctly identifies when WAR exceeds team cap +3. Edge cases around None values and floating point tolerance + +Why these tests matter: +- Salary cap validation is critical for league integrity during trades/drafts +- The helper functions centralize logic previously scattered across commands +- Proper fallback behavior ensures backwards compatibility +""" + +import pytest +from utils.helpers import ( + DEFAULT_SALARY_CAP, + SALARY_CAP_TOLERANCE, + get_team_salary_cap, + exceeds_salary_cap +) + + +class TestGetTeamSalaryCap: + """Tests for get_team_salary_cap() function.""" + + def test_returns_team_salary_cap_when_set(self): + """ + When a team has a custom salary_cap value set, return that value. + + Why: Some teams may have different caps (expansion teams, penalties, etc.) + """ + team = {'abbrev': 'TEST', 'salary_cap': 35.0} + result = get_team_salary_cap(team) + assert result == 35.0 + + def test_returns_default_when_salary_cap_is_none(self): + """ + When team.salary_cap is None, return the default cap (32.0). + + Why: Most teams use the standard cap; None indicates no custom value. + """ + team = {'abbrev': 'TEST', 'salary_cap': None} + result = get_team_salary_cap(team) + assert result == DEFAULT_SALARY_CAP + assert result == 32.0 + + def test_returns_default_when_salary_cap_key_missing(self): + """ + When the salary_cap key doesn't exist in team dict, return default. + + Why: Backwards compatibility with older team data structures. + """ + team = {'abbrev': 'TEST', 'sname': 'Test Team'} + result = get_team_salary_cap(team) + assert result == DEFAULT_SALARY_CAP + + def test_returns_default_when_team_is_none(self): + """ + When team is None, return the default cap. + + Why: Defensive programming - callers may pass None in edge cases. + """ + result = get_team_salary_cap(None) + assert result == DEFAULT_SALARY_CAP + + def test_returns_default_when_team_is_empty_dict(self): + """ + When team is an empty dict, return the default cap. + + Why: Edge case handling for malformed team data. + """ + result = get_team_salary_cap({}) + assert result == DEFAULT_SALARY_CAP + + def test_respects_zero_salary_cap(self): + """ + When salary_cap is explicitly 0, return 0 (not default). + + Why: Zero is a valid value (e.g., suspended team), distinct from None. + """ + team = {'abbrev': 'BANNED', 'salary_cap': 0.0} + result = get_team_salary_cap(team) + assert result == 0.0 + + def test_handles_integer_salary_cap(self): + """ + When salary_cap is an integer, return it as-is. + + Why: API may return int instead of float; function should handle both. + """ + team = {'abbrev': 'TEST', 'salary_cap': 30} + result = get_team_salary_cap(team) + assert result == 30 + + +class TestExceedsSalaryCap: + """Tests for exceeds_salary_cap() function.""" + + def test_returns_false_when_under_cap(self): + """ + WAR of 30.0 should not exceed default cap of 32.0. + + Why: Normal case - team is under cap and should pass validation. + """ + team = {'abbrev': 'TEST', 'salary_cap': 32.0} + result = exceeds_salary_cap(30.0, team) + assert result is False + + def test_returns_false_when_exactly_at_cap(self): + """ + WAR of exactly 32.0 should not exceed cap (within tolerance). + + Why: Teams should be allowed to be exactly at cap limit. + """ + team = {'abbrev': 'TEST', 'salary_cap': 32.0} + result = exceeds_salary_cap(32.0, team) + assert result is False + + def test_returns_false_within_tolerance(self): + """ + WAR slightly above cap but within tolerance should not exceed. + + Why: Floating point math may produce values like 32.0000001; + tolerance prevents false positives from rounding errors. + """ + team = {'abbrev': 'TEST', 'salary_cap': 32.0} + # 32.0005 is within 0.001 tolerance of 32.0 + result = exceeds_salary_cap(32.0005, team) + assert result is False + + def test_returns_true_when_over_cap(self): + """ + WAR of 33.0 clearly exceeds cap of 32.0. + + Why: Core validation - must reject teams over cap. + """ + team = {'abbrev': 'TEST', 'salary_cap': 32.0} + result = exceeds_salary_cap(33.0, team) + assert result is True + + def test_returns_true_just_over_tolerance(self): + """ + WAR just beyond tolerance should exceed cap. + + Why: Tolerance has a boundary; values beyond it must fail. + """ + team = {'abbrev': 'TEST', 'salary_cap': 32.0} + # 32.002 is beyond 0.001 tolerance + result = exceeds_salary_cap(32.002, team) + assert result is True + + def test_uses_team_custom_cap(self): + """ + Should use team's custom cap, not default. + + Why: Teams with higher/lower caps must be validated correctly. + """ + team = {'abbrev': 'EXPANSION', 'salary_cap': 28.0} + # 30.0 is under default 32.0 but over custom 28.0 + result = exceeds_salary_cap(30.0, team) + assert result is True + + def test_uses_default_cap_when_team_cap_none(self): + """ + When team has no custom cap, use default for comparison. + + Why: Backwards compatibility - existing teams without salary_cap field. + """ + team = {'abbrev': 'TEST', 'salary_cap': None} + result = exceeds_salary_cap(33.0, team) + assert result is True + + result = exceeds_salary_cap(31.0, team) + assert result is False + + def test_handles_none_team(self): + """ + When team is None, use default cap for comparison. + + Why: Defensive programming for edge cases. + """ + result = exceeds_salary_cap(33.0, None) + assert result is True + + result = exceeds_salary_cap(31.0, None) + assert result is False + + +class TestPydanticModelSupport: + """Tests for Pydantic model support in helper functions.""" + + def test_get_team_salary_cap_with_pydantic_model(self): + """ + Should work with Pydantic models that have salary_cap attribute. + + Why: Team objects in the codebase are often Pydantic models, + not just dicts. The helper must support both. + """ + class MockTeam: + salary_cap = 35.0 + abbrev = 'TEST' + + team = MockTeam() + result = get_team_salary_cap(team) + assert result == 35.0 + + def test_get_team_salary_cap_with_pydantic_model_none_cap(self): + """ + Pydantic model with salary_cap=None should return default. + + Why: Many existing Team objects have salary_cap=None. + """ + class MockTeam: + salary_cap = None + abbrev = 'TEST' + + team = MockTeam() + result = get_team_salary_cap(team) + assert result == DEFAULT_SALARY_CAP + + def test_get_team_salary_cap_with_object_missing_attribute(self): + """ + Object without salary_cap attribute should return default. + + Why: Defensive handling for objects that don't have the attribute. + """ + class MockTeam: + abbrev = 'TEST' + + team = MockTeam() + result = get_team_salary_cap(team) + assert result == DEFAULT_SALARY_CAP + + def test_exceeds_salary_cap_with_pydantic_model(self): + """ + exceeds_salary_cap should work with Pydantic-like objects. + + Why: Draft and transaction code passes Team objects directly. + """ + class MockTeam: + salary_cap = 28.0 + abbrev = 'EXPANSION' + + team = MockTeam() + # 30.0 exceeds custom cap of 28.0 + result = exceeds_salary_cap(30.0, team) + assert result is True + + # 27.0 does not exceed custom cap of 28.0 + result = exceeds_salary_cap(27.0, team) + assert result is False + + +class TestEdgeCases: + """Tests for edge cases and boundary conditions.""" + + def test_negative_salary_cap(self): + """ + Negative salary cap should be returned as-is (even if nonsensical). + + Why: Function should not validate business logic - just return the value. + If someone sets a negative cap, that's a data issue, not a helper issue. + """ + team = {'abbrev': 'BROKE', 'salary_cap': -5.0} + result = get_team_salary_cap(team) + assert result == -5.0 + + def test_negative_wara_under_cap(self): + """ + Negative WAR should not exceed any positive cap. + + Why: Teams with negative WAR (all bad players) are clearly under cap. + """ + team = {'abbrev': 'TEST', 'salary_cap': 32.0} + result = exceeds_salary_cap(-10.0, team) + assert result is False + + def test_negative_wara_with_negative_cap(self): + """ + Negative WAR vs negative cap - WAR higher than cap exceeds it. + + Why: Edge case where both values are negative. -3.0 > -5.0 + 0.001 + """ + team = {'abbrev': 'BROKE', 'salary_cap': -5.0} + # -3.0 > -4.999 (which is -5.0 + 0.001), so it exceeds + result = exceeds_salary_cap(-3.0, team) + assert result is True + + # -6.0 < -4.999, so it does not exceed + result = exceeds_salary_cap(-6.0, team) + assert result is False + + def test_very_large_salary_cap(self): + """ + Very large salary cap values should work correctly. + + Why: Ensure no overflow or precision issues with large numbers. + """ + team = {'abbrev': 'RICH', 'salary_cap': 1000000.0} + result = get_team_salary_cap(team) + assert result == 1000000.0 + + result = exceeds_salary_cap(999999.0, team) + assert result is False + + result = exceeds_salary_cap(1000001.0, team) + assert result is True + + def test_very_small_salary_cap(self): + """ + Very small (but positive) salary cap should work. + + Why: Some hypothetical penalty scenario with tiny cap. + """ + team = {'abbrev': 'TINY', 'salary_cap': 0.5} + result = exceeds_salary_cap(0.4, team) + assert result is False + + result = exceeds_salary_cap(0.6, team) + assert result is True + + def test_float_precision_boundary(self): + """ + Test exact boundary of tolerance (cap + 0.001). + + Why: Ensure the boundary condition is handled correctly. + The check is wara > (cap + tolerance), so exactly at boundary should NOT exceed. + """ + team = {'abbrev': 'TEST', 'salary_cap': 32.0} + # Exactly at cap + tolerance = 32.001 + result = exceeds_salary_cap(32.001, team) + assert result is False # Not greater than, equal to + + # Just barely over + result = exceeds_salary_cap(32.0011, team) + assert result is True + + +class TestRealTeamModel: + """Tests using the actual Team Pydantic model from models/team.py.""" + + def test_with_real_team_model(self): + """ + Test with the actual Team Pydantic model used in production. + + Why: Ensures the helper works with real Team objects, not just mocks. + """ + from models.team import Team + + team = Team( + id=1, + abbrev='TEST', + sname='Test Team', + lname='Test Team Long Name', + season=12, + salary_cap=28.5 + ) + result = get_team_salary_cap(team) + assert result == 28.5 + + def test_with_real_team_model_none_cap(self): + """ + Real Team model with salary_cap=None should use default. + + Why: This is the most common case in production. + """ + from models.team import Team + + team = Team( + id=2, + abbrev='STD', + sname='Standard Team', + lname='Standard Team Long Name', + season=12, + salary_cap=None + ) + result = get_team_salary_cap(team) + assert result == DEFAULT_SALARY_CAP + + def test_exceeds_with_real_team_model(self): + """ + exceeds_salary_cap with real Team model. + + Why: End-to-end test with actual production model. + """ + from models.team import Team + + team = Team( + id=3, + abbrev='EXP', + sname='Expansion', + lname='Expansion Team', + season=12, + salary_cap=28.0 + ) + # 30.0 exceeds 28.0 cap + assert exceeds_salary_cap(30.0, team) is True + # 27.0 does not exceed 28.0 cap + assert exceeds_salary_cap(27.0, team) is False + + +class TestConstants: + """Tests for salary cap constants.""" + + def test_default_salary_cap_value(self): + """ + DEFAULT_SALARY_CAP should be 32.0 (league standard). + + Why: Ensures constant wasn't accidentally changed. + """ + assert DEFAULT_SALARY_CAP == 32.0 + + def test_tolerance_value(self): + """ + SALARY_CAP_TOLERANCE should be 0.001. + + Why: Tolerance must be small enough to catch real violations + but large enough to handle floating point imprecision. + """ + assert SALARY_CAP_TOLERANCE == 0.001 diff --git a/utils/draft_helpers.py b/utils/draft_helpers.py index 8cd4386..60dbc1c 100644 --- a/utils/draft_helpers.py +++ b/utils/draft_helpers.py @@ -110,15 +110,16 @@ def calculate_overall_from_round_position(round_num: int, position: int) -> int: async def validate_cap_space( roster: dict, - new_player_wara: float -) -> Tuple[bool, float]: + new_player_wara: float, + team=None +) -> Tuple[bool, float, float]: """ Validate team has cap space to draft player. Cap calculation: - Maximum 32 players on active roster - Only top 26 players count toward cap - - Cap limit: 32.00 sWAR total + - Cap limit: Team-specific or default 32.00 sWAR Args: roster: Roster dictionary from API with structure: @@ -129,15 +130,18 @@ async def validate_cap_space( } } new_player_wara: sWAR value of player being drafted + team: Optional team object/dict for team-specific salary cap Returns: - (valid, projected_total): True if under cap, projected total sWAR after addition + (valid, projected_total, cap_limit): True if under cap, projected total sWAR, and cap limit used Raises: ValueError: If roster structure is invalid """ + from utils.helpers import get_team_salary_cap, SALARY_CAP_TOLERANCE + config = get_config() - cap_limit = config.swar_cap_limit + cap_limit = get_team_salary_cap(team) cap_player_count = config.cap_player_count if not roster or not roster.get('active'): @@ -150,31 +154,34 @@ async def validate_cap_space( current_roster_size = len(current_players) projected_roster_size = current_roster_size + 1 - # Maximum zeroes = 32 - roster size - # Maximum counted = 26 - zeroes - max_zeroes = 32 - projected_roster_size - max_counted = min(cap_player_count, cap_player_count - max_zeroes) # Can't count more than cap_player_count + # Cap counting rules: + # - The 26 CHEAPEST (lowest WAR) players on the roster count toward the cap + # - If roster has fewer than 26 players, all of them count + # - If roster has 26+ players, only the bottom 26 by WAR count + # - This allows expensive stars to be "excluded" if you have enough cheap depth + players_counted = min(projected_roster_size, cap_player_count) - # Sort all players (including new) by sWAR descending + # Sort all players (including new) by sWAR ASCENDING (cheapest first) all_players_wara = [p['wara'] for p in current_players] + [new_player_wara] - sorted_wara = sorted(all_players_wara, reverse=True) + sorted_wara = sorted(all_players_wara) # Ascending order - # Sum top N players - projected_total = sum(sorted_wara[:max_counted]) + # Sum bottom N players (the cheapest ones) + projected_total = sum(sorted_wara[:players_counted]) # Allow tiny floating point tolerance - is_valid = projected_total <= (cap_limit + 0.00001) + is_valid = projected_total <= (cap_limit + SALARY_CAP_TOLERANCE) logger.debug( f"Cap validation: roster_size={current_roster_size}, " f"projected_size={projected_roster_size}, " - f"max_counted={max_counted}, " + f"players_counted={players_counted}, " f"new_player_wara={new_player_wara:.2f}, " f"projected_total={projected_total:.2f}, " + f"cap_limit={cap_limit:.2f}, " f"valid={is_valid}" ) - return is_valid, projected_total + return is_valid, projected_total, cap_limit def format_pick_display(overall: int) -> str: diff --git a/utils/helpers.py b/utils/helpers.py new file mode 100644 index 0000000..ccdeaba --- /dev/null +++ b/utils/helpers.py @@ -0,0 +1,59 @@ +""" +Helper functions for Discord Bot v2.0 + +Contains utility functions for salary cap calculations and other common operations. +""" +from typing import Union +from config import get_config + +# Get default values from config +_config = get_config() + +# Salary cap constants - default from config, tolerance for float comparisons +DEFAULT_SALARY_CAP = _config.swar_cap_limit # 32.0 +SALARY_CAP_TOLERANCE = 0.001 # Small tolerance for floating point comparisons + + +def get_team_salary_cap(team) -> float: + """ + Get the salary cap for a team, falling back to the default if not set. + + Args: + team: Team data - can be a dict or Pydantic model with 'salary_cap' attribute. + + Returns: + float: The team's salary cap, or DEFAULT_SALARY_CAP (32.0) if not set. + + Why: Teams may have custom salary caps (e.g., for expansion teams or penalties). + This centralizes the fallback logic so all cap checks use the same source of truth. + """ + if team is None: + return DEFAULT_SALARY_CAP + + # Handle both dict and Pydantic model (or any object with salary_cap attribute) + if isinstance(team, dict): + salary_cap = team.get('salary_cap') + else: + salary_cap = getattr(team, 'salary_cap', None) + + if salary_cap is not None: + return salary_cap + return DEFAULT_SALARY_CAP + + +def exceeds_salary_cap(wara: float, team) -> bool: + """ + Check if a WAR total exceeds the team's salary cap. + + Args: + wara: The total WAR value to check + team: Team data - can be a dict or Pydantic model + + Returns: + bool: True if wara exceeds the team's salary cap (with tolerance) + + Why: Centralizes the salary cap comparison logic with proper floating point + tolerance handling. All cap validation should use this function. + """ + cap = get_team_salary_cap(team) + return wara > (cap + SALARY_CAP_TOLERANCE) diff --git a/views/draft_views.py b/views/draft_views.py index 9ee08d6..8a1fad9 100644 --- a/views/draft_views.py +++ b/views/draft_views.py @@ -67,10 +67,11 @@ async def create_on_the_clock_embed( # Add team sWAR if provided if team_roster_swar is not None: - config = get_config() + from utils.helpers import get_team_salary_cap + cap_limit = get_team_salary_cap(current_pick.owner) embed.add_field( name="Current sWAR", - value=f"{team_roster_swar:.2f} / {config.swar_cap_limit:.2f}", + value=f"{team_roster_swar:.2f} / {cap_limit:.2f}", inline=True ) @@ -345,7 +346,8 @@ async def create_pick_success_embed( player: Player, team: Team, pick_overall: int, - projected_swar: float + projected_swar: float, + cap_limit: float = None ) -> discord.Embed: """ Create embed for successful pick. @@ -355,10 +357,13 @@ async def create_pick_success_embed( team: Team that drafted player pick_overall: Overall pick number projected_swar: Projected team sWAR after pick + cap_limit: Team's salary cap limit (optional, uses helper if not provided) Returns: Discord success embed """ + from utils.helpers import get_team_salary_cap + embed = EmbedTemplate.success( title="Pick Confirmed", description=f"{team.abbrev} selects **{player.name}**" @@ -377,10 +382,13 @@ async def create_pick_success_embed( inline=True ) - config = get_config() + # Use provided cap_limit or get from team + if cap_limit is None: + cap_limit = get_team_salary_cap(team) + embed.add_field( name="Projected Team sWAR", - value=f"{projected_swar:.2f} / {config.swar_cap_limit:.2f}", + value=f"{projected_swar:.2f} / {cap_limit:.2f}", inline=True )