diff --git a/commands/draft/CLAUDE.md b/commands/draft/CLAUDE.md index 1313318..bc912d1 100644 --- a/commands/draft/CLAUDE.md +++ b/commands/draft/CLAUDE.md @@ -52,6 +52,25 @@ Draft commands are only available in the offseason. ## 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 - **Purpose**: Prevent concurrent draft picks that could cause race conditions - **Implementation**: `asyncio.Lock()` stored in cog instance diff --git a/commands/draft/picks.py b/commands/draft/picks.py index e0366db..ff28ad6 100644 --- a/commands/draft/picks.py +++ b/commands/draft/picks.py @@ -173,15 +173,32 @@ class DraftPicksCog(commands.Cog): await interaction.followup.send(embed=embed) 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: - # TODO: Check for skipped picks - embed = await create_pick_illegal_embed( - "Not Your Turn", - f"{current_pick.owner.sname} is on the clock for {format_pick_display(current_pick.overall)}." + # 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( + "Not Your Turn", + f"{current_pick.owner.sname} is on the clock for {format_pick_display(current_pick.overall)}." + ) + await interaction.followup.send(embed=embed) + 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})" ) - await interaction.followup.send(embed=embed) - return # Get player 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) 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( - current_pick.id, + pick_to_use.id, player_obj.id ) @@ -248,31 +265,50 @@ class DraftPicksCog(commands.Cog): if not updated_player: 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 success_embed = await create_pick_success_embed( player_obj, team, - current_pick.overall, + pick_to_use.overall, projected_total, 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) - # Post draft card to ping channel - if draft_data.ping_channel: + # Post draft card to ping channel (only if different from command channel) + if draft_data.ping_channel and draft_data.ping_channel != interaction.channel_id: guild = interaction.guild if guild: ping_channel = guild.get_channel(draft_data.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) - # Advance to next pick - await draft_service.advance_pick(draft_data.id, draft_data.currentpick) + # 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) self.logger.info( 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 "") ) diff --git a/services/draft_pick_service.py b/services/draft_pick_service.py index a081998..2d3a757 100644 --- a/services/draft_pick_service.py +++ b/services/draft_pick_service.py @@ -208,6 +208,52 @@ class DraftPickService(BaseService[DraftPick]): logger.error(f"Error getting available picks: {e}") 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( self, season: int, diff --git a/tests/test_services_draft.py b/tests/test_services_draft.py index 938c713..9ab58f1 100644 --- a/tests/test_services_draft.py +++ b/tests/test_services_draft.py @@ -775,6 +775,89 @@ class TestDraftPickService: assert patch_data['player_id'] is None 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