Add skipped pick support to /draft command
Teams can now make up missed draft picks when not on the clock: - Added get_skipped_picks_for_team() to draft_pick_service - /draft checks for skipped picks when user isn't on the clock - Uses earliest (lowest overall) skipped pick first - Shows footer noting "Making up skipped pick" on success - Does NOT advance draft when using skipped pick - Fixed duplicate embed when command run in ping channel 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
ada4feef3e
commit
8f486a8424
@ -52,6 +52,25 @@ Draft commands are only available in the offseason.
|
|||||||
|
|
||||||
## Key Features
|
## Key Features
|
||||||
|
|
||||||
|
### Skipped Pick Support
|
||||||
|
- **Purpose**: Allow teams to make up picks they missed when not on the clock
|
||||||
|
- **Detection**: Checks for picks with `overall < current_overall` and `player_id = None`
|
||||||
|
- **Behavior**: If team is not on the clock but has skipped picks, allows drafting with earliest skipped pick
|
||||||
|
- **User Experience**: Success message includes footer noting this is a "skipped pick makeup"
|
||||||
|
- **Draft Advancement**: Does NOT advance the draft when using a skipped pick
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Skipped pick detection flow
|
||||||
|
if current_pick.owner.id != team.id:
|
||||||
|
skipped_picks = await draft_pick_service.get_skipped_picks_for_team(
|
||||||
|
season, team.id, current_overall
|
||||||
|
)
|
||||||
|
if skipped_picks:
|
||||||
|
pick_to_use = skipped_picks[0] # Earliest skipped pick
|
||||||
|
else:
|
||||||
|
# Return "Not Your Turn" error
|
||||||
|
```
|
||||||
|
|
||||||
### Global Pick Lock
|
### Global Pick Lock
|
||||||
- **Purpose**: Prevent concurrent draft picks that could cause race conditions
|
- **Purpose**: Prevent concurrent draft picks that could cause race conditions
|
||||||
- **Implementation**: `asyncio.Lock()` stored in cog instance
|
- **Implementation**: `asyncio.Lock()` stored in cog instance
|
||||||
|
|||||||
@ -173,9 +173,19 @@ class DraftPicksCog(commands.Cog):
|
|||||||
await interaction.followup.send(embed=embed)
|
await interaction.followup.send(embed=embed)
|
||||||
return
|
return
|
||||||
|
|
||||||
# Validate user is on the clock
|
# Validate user is on the clock OR has a skipped pick
|
||||||
|
pick_to_use = current_pick # Default: use current pick if on the clock
|
||||||
|
|
||||||
if current_pick.owner.id != team.id:
|
if current_pick.owner.id != team.id:
|
||||||
# TODO: Check for skipped picks
|
# Not on the clock - check for skipped picks
|
||||||
|
skipped_picks = await draft_pick_service.get_skipped_picks_for_team(
|
||||||
|
config.sba_season,
|
||||||
|
team.id,
|
||||||
|
draft_data.currentpick
|
||||||
|
)
|
||||||
|
|
||||||
|
if not skipped_picks:
|
||||||
|
# No skipped picks - can't draft
|
||||||
embed = await create_pick_illegal_embed(
|
embed = await create_pick_illegal_embed(
|
||||||
"Not Your Turn",
|
"Not Your Turn",
|
||||||
f"{current_pick.owner.sname} is on the clock for {format_pick_display(current_pick.overall)}."
|
f"{current_pick.owner.sname} is on the clock for {format_pick_display(current_pick.overall)}."
|
||||||
@ -183,6 +193,13 @@ class DraftPicksCog(commands.Cog):
|
|||||||
await interaction.followup.send(embed=embed)
|
await interaction.followup.send(embed=embed)
|
||||||
return
|
return
|
||||||
|
|
||||||
|
# Use the earliest skipped pick
|
||||||
|
pick_to_use = skipped_picks[0]
|
||||||
|
self.logger.info(
|
||||||
|
f"Team {team.abbrev} using skipped pick #{pick_to_use.overall} "
|
||||||
|
f"(current pick is #{current_pick.overall})"
|
||||||
|
)
|
||||||
|
|
||||||
# Get player
|
# Get player
|
||||||
players = await player_service.get_players_by_name(player_name, config.sba_season)
|
players = await player_service.get_players_by_name(player_name, config.sba_season)
|
||||||
|
|
||||||
@ -225,9 +242,9 @@ class DraftPicksCog(commands.Cog):
|
|||||||
await interaction.followup.send(embed=embed)
|
await interaction.followup.send(embed=embed)
|
||||||
return
|
return
|
||||||
|
|
||||||
# Execute pick
|
# Execute pick (using pick_to_use which may be current or skipped pick)
|
||||||
updated_pick = await draft_pick_service.update_pick_selection(
|
updated_pick = await draft_pick_service.update_pick_selection(
|
||||||
current_pick.id,
|
pick_to_use.id,
|
||||||
player_obj.id
|
player_obj.id
|
||||||
)
|
)
|
||||||
|
|
||||||
@ -248,31 +265,50 @@ class DraftPicksCog(commands.Cog):
|
|||||||
if not updated_player:
|
if not updated_player:
|
||||||
self.logger.error(f"Failed to update player {player_obj.id} team")
|
self.logger.error(f"Failed to update player {player_obj.id} team")
|
||||||
|
|
||||||
|
# Determine if this was a skipped pick
|
||||||
|
is_skipped_pick = pick_to_use.overall != current_pick.overall
|
||||||
|
|
||||||
# Send success message
|
# Send success message
|
||||||
success_embed = await create_pick_success_embed(
|
success_embed = await create_pick_success_embed(
|
||||||
player_obj,
|
player_obj,
|
||||||
team,
|
team,
|
||||||
current_pick.overall,
|
pick_to_use.overall,
|
||||||
projected_total,
|
projected_total,
|
||||||
cap_limit
|
cap_limit
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Add note if this was a skipped pick
|
||||||
|
if is_skipped_pick:
|
||||||
|
success_embed.set_footer(
|
||||||
|
text=f"📝 Making up skipped pick (current pick is #{current_pick.overall})"
|
||||||
|
)
|
||||||
|
|
||||||
await interaction.followup.send(embed=success_embed)
|
await interaction.followup.send(embed=success_embed)
|
||||||
|
|
||||||
# Post draft card to ping channel
|
# Post draft card to ping channel (only if different from command channel)
|
||||||
if draft_data.ping_channel:
|
if draft_data.ping_channel and draft_data.ping_channel != interaction.channel_id:
|
||||||
guild = interaction.guild
|
guild = interaction.guild
|
||||||
if guild:
|
if guild:
|
||||||
ping_channel = guild.get_channel(draft_data.ping_channel)
|
ping_channel = guild.get_channel(draft_data.ping_channel)
|
||||||
if ping_channel:
|
if ping_channel:
|
||||||
draft_card = await create_player_draft_card(player_obj, current_pick)
|
draft_card = await create_player_draft_card(player_obj, pick_to_use)
|
||||||
|
|
||||||
|
# Add skipped pick context to draft card
|
||||||
|
if is_skipped_pick:
|
||||||
|
draft_card.set_footer(
|
||||||
|
text=f"📝 Making up skipped pick (current pick is #{current_pick.overall})"
|
||||||
|
)
|
||||||
|
|
||||||
await ping_channel.send(embed=draft_card)
|
await ping_channel.send(embed=draft_card)
|
||||||
|
|
||||||
# Advance to next pick
|
# Only advance the draft if this was the current pick (not a skipped pick)
|
||||||
|
if not is_skipped_pick:
|
||||||
await draft_service.advance_pick(draft_data.id, draft_data.currentpick)
|
await draft_service.advance_pick(draft_data.id, draft_data.currentpick)
|
||||||
|
|
||||||
self.logger.info(
|
self.logger.info(
|
||||||
f"Draft pick completed: {team.abbrev} selected {player_obj.name} "
|
f"Draft pick completed: {team.abbrev} selected {player_obj.name} "
|
||||||
f"(pick #{current_pick.overall})"
|
f"(pick #{pick_to_use.overall})"
|
||||||
|
+ (f" [skipped pick makeup]" if is_skipped_pick else "")
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@ -208,6 +208,52 @@ class DraftPickService(BaseService[DraftPick]):
|
|||||||
logger.error(f"Error getting available picks: {e}")
|
logger.error(f"Error getting available picks: {e}")
|
||||||
return []
|
return []
|
||||||
|
|
||||||
|
async def get_skipped_picks_for_team(
|
||||||
|
self,
|
||||||
|
season: int,
|
||||||
|
team_id: int,
|
||||||
|
current_overall: int
|
||||||
|
) -> List[DraftPick]:
|
||||||
|
"""
|
||||||
|
Get skipped picks for a team (picks before current that have no player selected).
|
||||||
|
|
||||||
|
A "skipped" pick is one where:
|
||||||
|
- The pick overall is LESS than the current overall (it has passed)
|
||||||
|
- The pick has no player_id assigned
|
||||||
|
- The pick's current owner is the specified team
|
||||||
|
|
||||||
|
NOT cached - picks change during draft.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
season: Draft season
|
||||||
|
team_id: Team ID to check for skipped picks
|
||||||
|
current_overall: Current overall pick number in the draft
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
List of skipped DraftPick instances owned by team, ordered by overall (ascending)
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
# Get all picks owned by this team that are before the current pick
|
||||||
|
# and have not been selected
|
||||||
|
params = [
|
||||||
|
('season', str(season)),
|
||||||
|
('owner_team_id', str(team_id)),
|
||||||
|
('overall_end', str(current_overall - 1)), # Before current pick
|
||||||
|
('player_taken', 'false'), # No player selected
|
||||||
|
('sort', 'order-asc') # Earliest skipped pick first
|
||||||
|
]
|
||||||
|
|
||||||
|
picks = await self.get_all_items(params=params)
|
||||||
|
logger.debug(
|
||||||
|
f"Found {len(picks)} skipped picks for team {team_id} "
|
||||||
|
f"before pick #{current_overall}"
|
||||||
|
)
|
||||||
|
return picks
|
||||||
|
|
||||||
|
except Exception as e:
|
||||||
|
logger.error(f"Error getting skipped picks for team {team_id}: {e}")
|
||||||
|
return []
|
||||||
|
|
||||||
async def get_recent_picks(
|
async def get_recent_picks(
|
||||||
self,
|
self,
|
||||||
season: int,
|
season: int,
|
||||||
|
|||||||
@ -775,6 +775,89 @@ class TestDraftPickService:
|
|||||||
assert patch_data['player_id'] is None
|
assert patch_data['player_id'] is None
|
||||||
assert 'overall' in patch_data # Full model required
|
assert 'overall' in patch_data # Full model required
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_get_skipped_picks_for_team_success(self, service, mock_client):
|
||||||
|
"""
|
||||||
|
Test retrieving skipped picks for a team.
|
||||||
|
|
||||||
|
Skipped picks are picks before the current overall that have no player selected.
|
||||||
|
Returns picks ordered by overall (ascending) so earliest skipped pick is first.
|
||||||
|
"""
|
||||||
|
# Team 5 has two skipped picks (overall 10 and 15) before current pick 25
|
||||||
|
skipped_pick_1 = create_draft_pick_data(
|
||||||
|
pick_id=10, overall=10, round_num=1, player_id=None,
|
||||||
|
owner_team_id=5, include_nested=False
|
||||||
|
)
|
||||||
|
skipped_pick_2 = create_draft_pick_data(
|
||||||
|
pick_id=15, overall=15, round_num=1, player_id=None,
|
||||||
|
owner_team_id=5, include_nested=False
|
||||||
|
)
|
||||||
|
|
||||||
|
mock_client.get.return_value = {
|
||||||
|
'count': 2,
|
||||||
|
'picks': [skipped_pick_1, skipped_pick_2]
|
||||||
|
}
|
||||||
|
|
||||||
|
result = await service.get_skipped_picks_for_team(
|
||||||
|
season=12,
|
||||||
|
team_id=5,
|
||||||
|
current_overall=25
|
||||||
|
)
|
||||||
|
|
||||||
|
# Verify results
|
||||||
|
assert len(result) == 2
|
||||||
|
assert result[0].overall == 10 # Earliest skipped pick first
|
||||||
|
assert result[1].overall == 15
|
||||||
|
assert result[0].player_id is None
|
||||||
|
assert result[1].player_id is None
|
||||||
|
|
||||||
|
# Verify API call
|
||||||
|
mock_client.get.assert_called_once()
|
||||||
|
call_args = mock_client.get.call_args
|
||||||
|
params = call_args[1]['params']
|
||||||
|
# Should request picks before current (overall_end=24), owned by team, with no player
|
||||||
|
assert ('overall_end', '24') in params
|
||||||
|
assert ('owner_team_id', '5') in params
|
||||||
|
assert ('player_taken', 'false') in params
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_get_skipped_picks_for_team_none_found(self, service, mock_client):
|
||||||
|
"""
|
||||||
|
Test when team has no skipped picks.
|
||||||
|
|
||||||
|
Returns empty list when all prior picks have been made.
|
||||||
|
"""
|
||||||
|
mock_client.get.return_value = {
|
||||||
|
'count': 0,
|
||||||
|
'picks': []
|
||||||
|
}
|
||||||
|
|
||||||
|
result = await service.get_skipped_picks_for_team(
|
||||||
|
season=12,
|
||||||
|
team_id=5,
|
||||||
|
current_overall=25
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result == []
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_get_skipped_picks_for_team_api_error(self, service, mock_client):
|
||||||
|
"""
|
||||||
|
Test graceful handling of API errors.
|
||||||
|
|
||||||
|
Returns empty list on error rather than raising exception.
|
||||||
|
"""
|
||||||
|
mock_client.get.side_effect = Exception("API Error")
|
||||||
|
|
||||||
|
result = await service.get_skipped_picks_for_team(
|
||||||
|
season=12,
|
||||||
|
team_id=5,
|
||||||
|
current_overall=25
|
||||||
|
)
|
||||||
|
|
||||||
|
# Should return empty list on error, not raise
|
||||||
|
assert result == []
|
||||||
|
|
||||||
|
|
||||||
# =============================================================================
|
# =============================================================================
|
||||||
# DraftListService Tests
|
# DraftListService Tests
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user