Replace hardcoded salary cap with dynamic Team.salary_cap
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 <noreply@anthropic.com>
This commit is contained in:
parent
cd8cf0aee8
commit
bbb4233b45
@ -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'
|
||||
|
||||
@ -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](<https://www.bing.com/>) and for [Sheets](<https://docs.google.com/spreadsheets/u/0/>).\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}')
|
||||
|
||||
21
helpers.py
21
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)
|
||||
|
||||
@ -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"
|
||||
]
|
||||
}
|
||||
|
||||
@ -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."""
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user