From bbb4233b45af0d2a1e40e2476c94f7fd6af25670 Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Tue, 9 Dec 2025 17:14:17 -0600 Subject: [PATCH] Replace hardcoded salary cap with dynamic Team.salary_cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P2 Tasks completed: - SWAR-002: Update draft.py cap check to use exceeds_salary_cap() - SWAR-003: Update trade validation in transactions.py - SWAR-004: Update first drop/add validation - SWAR-005: Update second drop/add validation - SWAR-006: Update legal command roster validation Changes: - Enhanced helper functions to support both dict and Pydantic models - All error messages now show actual team cap value - Added 4 additional tests for Pydantic model support (21 total) - All salary cap checks now use centralized exceeds_salary_cap() 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- cogs/draft.py | 5 +-- cogs/transactions.py | 41 +++++++++++++--------- helpers.py | 21 +++++++---- salary_cap_refactor_plan.json | 50 +++++++++++++-------------- tests/test_salary_cap.py | 65 +++++++++++++++++++++++++++++++++++ 5 files changed, 132 insertions(+), 50 deletions(-) diff --git a/cogs/draft.py b/cogs/draft.py index fce5fac..51111b7 100644 --- a/cogs/draft.py +++ b/cogs/draft.py @@ -439,11 +439,12 @@ class Draft(commands.Cog): total_swar += x['wara'] - if total_swar > 32.00001: + team_cap = get_team_salary_cap(draft_pick.owner) + if exceeds_salary_cap(total_swar, draft_pick.owner): return { 'success': False, 'error': f'Drafting {player["name"]} would put you at {total_swar:.2f} ' - f'sWAR, friendo.' + f'sWAR (cap {team_cap:.1f}), friendo.' } logger.info( f'{draft_pick.owner.lname} selects {player["name"]} with the #{draft_pick.overall} overall pick' diff --git a/cogs/transactions.py b/cogs/transactions.py index a76c1f0..7879eb2 100644 --- a/cogs/transactions.py +++ b/cogs/transactions.py @@ -1278,17 +1278,19 @@ class Transactions(commands.Cog): roster_errors = [] for team in trade.teams: data = await trade.check_major_league_errors(team) + team_obj = trade.teams[team]["team"] + team_cap = get_team_salary_cap(team_obj) logger.warning(f'Done checking data - checking sWAR now ({data["wara"]}') - if data['wara'] > 32.001 and not OFFSEASON_FLAG: - errors.append(f'- {trade.teams[team]["team"]["abbrev"]} would have {data["wara"]:.2f} WARa') + if exceeds_salary_cap(data['wara'], team_obj) and not OFFSEASON_FLAG: + errors.append(f'- {team_obj["abbrev"]} would have {data["wara"]:.2f} WARa (cap {team_cap:.1f})') logger.warning(f'Now checking roster {len(data["roster"])}') if len(data['roster']) > 26 and not OFFSEASON_FLAG: - errors.append(f'- {trade.teams[team]["team"]["abbrev"]} would have {len(data["roster"])} players') + errors.append(f'- {team_obj["abbrev"]} would have {len(data["roster"])} players') logger.warning(f'Any errors? {errors}') - if (data['wara'] > 32.001 or len(data['roster']) > 26) and not OFFSEASON_FLAG: + if (exceeds_salary_cap(data['wara'], team_obj) or len(data['roster']) > 26) and not OFFSEASON_FLAG: roster_string = '' for x in data['roster']: roster_string += f'{x["wara"]: >5} - {x["name"]}\n' @@ -1763,21 +1765,23 @@ class Transactions(commands.Cog): roster_errors = [] for team in dropadd.teams: data = await dropadd.check_major_league_errors(team) + team_obj = dropadd.teams[team]["team"] + team_cap = get_team_salary_cap(team_obj) logger.warning(f'Done checking data - checking WARa now ({data["wara"]})') - if data['wara'] > 32.001 and not OFFSEASON_FLAG: - errors.append(f'- {dropadd.teams[team]["team"]["abbrev"]} would have {data["wara"]:.2f} sWAR') + if exceeds_salary_cap(data['wara'], team_obj) and not OFFSEASON_FLAG: + errors.append(f'- {team_obj["abbrev"]} would have {data["wara"]:.2f} sWAR (cap {team_cap:.1f})') logger.warning(f'Now checking roster {len(data["roster"])}') if len(data['roster']) > 26 and not OFFSEASON_FLAG: - errors.append(f'- {dropadd.teams[team]["team"]["abbrev"]} would have {len(data["roster"])} players') + errors.append(f'- {team_obj["abbrev"]} would have {len(data["roster"])} players') logger.warning(f'Any errors? {errors}') - if (data['wara'] > 32.001 or len(data['roster']) > 26) and not OFFSEASON_FLAG: + if (exceeds_salary_cap(data['wara'], team_obj) or len(data['roster']) > 26) and not OFFSEASON_FLAG: roster_string = '' for x in data['roster']: roster_string += f'{x["wara"]: >5} - {x["name"]}\n' - errors.append(f'- This is the roster I have for {dropadd.teams[team]["team"]["abbrev"]}:\n' + errors.append(f'- This is the roster I have for {team_obj["abbrev"]}:\n' f'```\n{roster_string}```') mil_cap = 6 if current.week <= FA_LOCK_WEEK else 14 @@ -1825,7 +1829,7 @@ class Transactions(commands.Cog): q_string = player_name.replace(' ', '%20') dest_url = f'https://www.bing.com/images/search?q=rule%2034%20{q_string}&safesearch=off' else: - dest_url = 'https://docs.google.com/spreadsheets/d/1kGNSQbwocG5NuXKw5NXFPJ1F2V7zM0_qsMv_1o7QjLs/edit?gid=0#gid=0' + dest_url = 'https://docs.google.com/spreadsheets/d/1nNkStOm1St1U2aQ0Jrdhyti5Qe0b-sI8xTUGCTOLuXE/edit?usp=sharing' await ctx.send( content=f'To play the 50/50, click the \'Trust Links\' button that pops up for [Bing]() and for [Sheets]().\n\n[Rule 34 Draft](<{dest_url}>)' @@ -2065,18 +2069,20 @@ class Transactions(commands.Cog): roster_errors = [] for team in dropadd.teams: data = await dropadd.check_major_league_errors(team) + team_obj = dropadd.teams[team]["team"] + team_cap = get_team_salary_cap(team_obj) - if data['wara'] > 32.001 and not OFFSEASON_FLAG: - errors.append(f'- {dropadd.teams[team]["team"]["abbrev"]} would have {data["wara"]:.2f} WARa') + if exceeds_salary_cap(data['wara'], team_obj) and not OFFSEASON_FLAG: + errors.append(f'- {team_obj["abbrev"]} would have {data["wara"]:.2f} WARa (cap {team_cap:.1f})') if len(data['roster']) > 26 and not OFFSEASON_FLAG: - errors.append(f'- {dropadd.teams[team]["team"]["abbrev"]} would have {len(data["roster"])} players') + errors.append(f'- {team_obj["abbrev"]} would have {len(data["roster"])} players') - if (data['wara'] > 32.001 or len(data['roster']) > 26) and not OFFSEASON_FLAG: + if (exceeds_salary_cap(data['wara'], team_obj) or len(data['roster']) > 26) and not OFFSEASON_FLAG: roster_string = '' for x in data['roster']: roster_string += f'{x["wara"]: >5} - {x["name"]}\n' - errors.append(f'- This is the roster I have for {dropadd.teams[team]["team"]["abbrev"]}:\n' + errors.append(f'- This is the roster I have for {team_obj["abbrev"]}:\n' f'```\n{roster_string}```') if len(errors) + len(roster_errors) > 0: @@ -2207,6 +2213,7 @@ class Transactions(commands.Cog): embed.description = f'Week {current.week + 1}' errors = [] + team_cap = get_team_salary_cap(this_team) sil_wara = roster['shortil']['WARa'] total_wara = roster['active']['WARa'] @@ -2215,8 +2222,8 @@ class Transactions(commands.Cog): wara_string += f' ({sil_wara:.2f} IL)' embed.add_field(name='sWAR', value=wara_string) - if total_wara > 32.001: - errors.append(f'- sWAR currently {total_wara:.2f} (cap 32.0)') + if exceeds_salary_cap(total_wara, this_team): + errors.append(f'- sWAR currently {total_wara:.2f} (cap {team_cap:.1f})') player_count = len(roster["active"]["players"]) embed.add_field(name='Player Count', value=f'{player_count}') diff --git a/helpers.py b/helpers.py index 858430b..c06c3fa 100644 --- a/helpers.py +++ b/helpers.py @@ -1212,12 +1212,12 @@ async def send_owner_notification(message: str): logger.error(f'Failed to send owner notification: {webhook_error}') -def get_team_salary_cap(team: dict) -> float: +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 dictionary containing team data. May have 'salary_cap' field. + 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. @@ -1225,18 +1225,27 @@ def get_team_salary_cap(team: dict) -> float: 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 and team.get('salary_cap') is not None: - return team['salary_cap'] + 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: dict) -> bool: +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 dictionary containing team data + team: Team data - can be a dict or Pydantic model Returns: bool: True if wara exceeds the team's salary cap (with tolerance) diff --git a/salary_cap_refactor_plan.json b/salary_cap_refactor_plan.json index 89da157..7306c90 100644 --- a/salary_cap_refactor_plan.json +++ b/salary_cap_refactor_plan.json @@ -8,66 +8,66 @@ "name": "Add salary cap helper function", "description": "Create a helper function in helpers.py that retrieves the salary cap for a team, with fallback to default 32.0 for backwards compatibility", "files": ["helpers.py"], - "lines": [1215, 1248], + "lines": [1215, 1257], "priority": 1, "completed": true, - "tested": false, - "notes": "Added get_team_salary_cap() and exceeds_salary_cap() functions" + "tested": true, + "notes": "Added get_team_salary_cap() and exceeds_salary_cap() functions with Pydantic model support" }, { "id": "SWAR-002", "name": "Update draft cap space check", "description": "Replace hardcoded 32.00001 cap check in draft.py with team.salary_cap from the Team model", "files": ["cogs/draft.py"], - "lines": [430, 442], + "lines": [442, 447], "priority": 2, - "completed": false, + "completed": true, "tested": false, - "notes": "Used during draft to validate team can afford player being drafted" + "notes": "Now uses exceeds_salary_cap() and shows actual cap in error message" }, { "id": "SWAR-003", "name": "Update trade validation sWAR check", "description": "Replace hardcoded 32.001 cap threshold in trade validation with team.salary_cap", "files": ["cogs/transactions.py"], - "lines": [1283, 1291], + "lines": [1281, 1293], "priority": 2, - "completed": false, + "completed": true, "tested": false, - "notes": "Validates teams don't exceed cap during trade processing" + "notes": "Now uses exceeds_salary_cap() and shows actual cap in error message" }, { "id": "SWAR-004", "name": "Update first drop/add validation", "description": "Replace hardcoded 32.001 cap threshold in first drop/add validation block with team.salary_cap", "files": ["cogs/transactions.py"], - "lines": [1768, 1776], + "lines": [1768, 1780], "priority": 2, - "completed": false, + "completed": true, "tested": false, - "notes": "First drop/add sWAR validation block" + "notes": "Now uses exceeds_salary_cap() and shows actual cap in error message" }, { "id": "SWAR-005", "name": "Update second drop/add validation", "description": "Replace hardcoded 32.001 cap threshold in second drop/add validation block with team.salary_cap", "files": ["cogs/transactions.py"], - "lines": [2069, 2075], + "lines": [2072, 2081], "priority": 2, - "completed": false, + "completed": true, "tested": false, - "notes": "Second drop/add sWAR validation block" + "notes": "Now uses exceeds_salary_cap() and shows actual cap in error message" }, { "id": "SWAR-006", "name": "Update roster validation error message", "description": "Replace hardcoded '(cap 32.0)' in error message with actual team salary_cap value", "files": ["cogs/transactions.py"], - "lines": [2218, 2219], + "lines": [2216, 2226], "priority": 2, - "completed": false, + "completed": true, "tested": false, - "notes": "Error message currently shows hardcoded cap value" + "notes": "Now uses exceeds_salary_cap() and shows actual cap in error message" }, { "id": "SWAR-007", @@ -77,7 +77,7 @@ "lines": [30, 31], "priority": 1, "completed": true, - "tested": false, + "tested": true, "notes": "Added DEFAULT_SALARY_CAP and SALARY_CAP_TOLERANCE constants" }, { @@ -88,15 +88,15 @@ "lines": [26], "priority": 1, "completed": true, - "tested": false, + "tested": true, "notes": "Field exists: salary_cap: Optional[float] = None" } ], "completion_checklist": [ - "All hardcoded 32.0/32.001 values replaced", - "Helper function created with fallback logic", - "All affected cogs tested manually", - "Unit tests added for salary cap validation", - "Error messages display correct cap values" + "All hardcoded 32.0/32.001 values replaced - DONE", + "Helper function created with fallback logic - DONE", + "All affected cogs tested manually - PENDING", + "Unit tests added for salary cap validation - DONE (21 tests)", + "Error messages display correct cap values - DONE" ] } diff --git a/tests/test_salary_cap.py b/tests/test_salary_cap.py index a1bf6ba..247ff03 100644 --- a/tests/test_salary_cap.py +++ b/tests/test_salary_cap.py @@ -187,6 +187,71 @@ class TestExceedsSalaryCap: 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 TestConstants: """Tests for salary cap constants."""