From f007c5b870115d50ba8588e3be41c2e5027085f5 Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Tue, 20 Jan 2026 13:35:48 -0600 Subject: [PATCH] Fix frozen flag bug and add Transaction Thaw Report for admins Bug Fix: - Fixed /dropadd transactions being marked frozen=True during thaw period - Now uses current_state.freeze to set frozen flag correctly - Transactions entered Sat-Sun are now unfrozen and execute Monday New Feature - Transaction Thaw Report: - Added data structures for thaw reporting (ThawReport, ThawedMove, CancelledMove, ConflictResolution, ConflictContender) - Modified resolve_contested_transactions() to return conflict details - Added _post_thaw_report() to post formatted report to admin channel - Report shows thawed moves, cancelled moves, and conflict resolution - Handles Discord's 2000 char limit with _send_long_report() Tests: - Updated test_views_transaction_embed.py for frozen flag behavior - Added test for thaw period (freeze=False) scenario - Updated test_tasks_transaction_freeze.py for new return values - All tests passing Co-Authored-By: Claude Opus 4.5 --- tasks/transaction_freeze.py | 309 ++++++++++++++++++++++++- tests/test_tasks_transaction_freeze.py | 31 ++- tests/test_views_transaction_embed.py | 42 +++- views/transaction_embed.py | 6 +- 4 files changed, 371 insertions(+), 17 deletions(-) diff --git a/tasks/transaction_freeze.py b/tasks/transaction_freeze.py index 4d283c6..37bf448 100644 --- a/tasks/transaction_freeze.py +++ b/tasks/transaction_freeze.py @@ -39,10 +39,64 @@ class TransactionPriority: return self.tiebreaker < other.tiebreaker +@dataclass +class ConflictContender: + """A team contending for a contested player.""" + team_abbrev: str + wins: int + losses: int + win_pct: float + move_id: str + + +@dataclass +class ConflictResolution: + """Details of a conflict resolution for a contested player.""" + player_name: str + player_swar: float + contenders: List[ConflictContender] + winner: ConflictContender + losers: List[ConflictContender] + + +@dataclass +class ThawedMove: + """A move that was successfully thawed (unfrozen).""" + move_id: str + team_abbrev: str + players: List[Tuple[str, float, str, str]] # (name, sWAR, old_team, new_team) + submitted_at: str # Extracted from moveid timestamp + + +@dataclass +class CancelledMove: + """A move that was cancelled due to conflict.""" + move_id: str + team_abbrev: str + players: List[Tuple[str, float, str, str]] # (name, sWAR, old_team, new_team) + lost_to: str # Team abbrev that won the contested player + contested_player: str # Name of player that caused the cancellation + + +@dataclass +class ThawReport: + """Complete thaw report for admin review.""" + week: int + season: int + timestamp: datetime + total_moves: int + thawed_count: int + cancelled_count: int + conflict_count: int + conflicts: List[ConflictResolution] + thawed_moves: List[ThawedMove] + cancelled_moves: List[CancelledMove] + + async def resolve_contested_transactions( transactions: List[Transaction], season: int -) -> Tuple[List[str], List[str]]: +) -> Tuple[List[str], List[str], List[ConflictResolution]]: """ Resolve contested transactions where multiple teams want the same player. @@ -53,7 +107,7 @@ async def resolve_contested_transactions( season: Current season number Returns: - Tuple of (winning_move_ids, losing_move_ids) + Tuple of (winning_move_ids, losing_move_ids, conflict_resolutions) """ logger = get_contextual_logger(f'{__name__}.resolve_contested_transactions') @@ -84,9 +138,12 @@ async def resolve_contested_transactions( # Resolve contests using team priority (win% + random tiebreaker) winning_move_ids: Set[str] = set() losing_move_ids: Set[str] = set() + conflict_resolutions: List[ConflictResolution] = [] for player_name, contested_transactions in contested_players.items(): priorities: List[TransactionPriority] = [] + # Track standings data for each team for report + team_standings_data: Dict[str, Tuple[int, int, float]] = {} # abbrev -> (wins, losses, win_pct) for transaction in contested_transactions: # Get team for priority calculation @@ -103,8 +160,12 @@ async def resolve_contested_transactions( if standings and standings.wins is not None and standings.losses is not None: total_games = standings.wins + standings.losses win_pct = standings.wins / total_games if total_games > 0 else 0.0 + team_standings_data[transaction.newteam.abbrev] = ( + standings.wins, standings.losses, win_pct + ) else: win_pct = 0.0 + team_standings_data[transaction.newteam.abbrev] = (0, 0, 0.0) logger.warning(f"Could not get standings for {team_abbrev}, using 0.0 win%") # Add small random component for tiebreaking (5 decimal precision) @@ -119,6 +180,7 @@ async def resolve_contested_transactions( except Exception as e: logger.error(f"Error calculating priority for {team_abbrev}: {e}") + team_standings_data[transaction.newteam.abbrev] = (0, 0, 0.0) # Give them 0.0 priority on error priorities.append(TransactionPriority( transaction=transaction, @@ -134,6 +196,20 @@ async def resolve_contested_transactions( winner = priorities[0] winning_move_ids.add(winner.transaction.moveid) + # Build conflict resolution record + winner_abbrev = winner.transaction.newteam.abbrev + winner_standings = team_standings_data.get(winner_abbrev, (0, 0, 0.0)) + winner_contender = ConflictContender( + team_abbrev=winner_abbrev, + wins=winner_standings[0], + losses=winner_standings[1], + win_pct=winner_standings[2], + move_id=winner.transaction.moveid + ) + + loser_contenders: List[ConflictContender] = [] + all_contenders: List[ConflictContender] = [winner_contender] + logger.info( f"Contest resolved for {player_name}: {winner.transaction.newteam.abbrev} wins " f"(win%: {winner.team_win_percentage:.3f}, tiebreaker: {winner.tiebreaker:.8f})" @@ -141,15 +217,37 @@ async def resolve_contested_transactions( for loser in priorities[1:]: losing_move_ids.add(loser.transaction.moveid) + loser_abbrev = loser.transaction.newteam.abbrev + loser_standings = team_standings_data.get(loser_abbrev, (0, 0, 0.0)) + loser_contender = ConflictContender( + team_abbrev=loser_abbrev, + wins=loser_standings[0], + losses=loser_standings[1], + win_pct=loser_standings[2], + move_id=loser.transaction.moveid + ) + loser_contenders.append(loser_contender) + all_contenders.append(loser_contender) + logger.info( f"Contest lost for {player_name}: {loser.transaction.newteam.abbrev} " f"(win%: {loser.team_win_percentage:.3f}, tiebreaker: {loser.tiebreaker:.8f})" ) + # Get player info from first transaction (they all have same player) + player = contested_transactions[0].player + conflict_resolutions.append(ConflictResolution( + player_name=player.name, + player_swar=player.wara, + contenders=all_contenders, + winner=winner_contender, + losers=loser_contenders + )) + # Add non-contested moves to winners winning_move_ids.update(non_contested_moves) - return list(winning_move_ids), list(losing_move_ids) + return list(winning_move_ids), list(losing_move_ids), conflict_resolutions class TransactionFreezeTask: @@ -408,6 +506,7 @@ class TransactionFreezeTask: 2. Resolve contested transactions (multiple teams want same player) 3. Cancel losing transactions 4. Unfreeze and post winning transactions + 5. Generate and post admin thaw report """ try: # Get all frozen transactions for current week via service @@ -419,16 +518,38 @@ class TransactionFreezeTask: if not transactions: self.logger.warning(f"No frozen transactions to process for week {current.week}") + # Still post an empty report for visibility + empty_report = ThawReport( + week=current.week, + season=current.season, + timestamp=datetime.now(UTC), + total_moves=0, + thawed_count=0, + cancelled_count=0, + conflict_count=0, + conflicts=[], + thawed_moves=[], + cancelled_moves=[] + ) + await self._post_thaw_report(empty_report) return self.logger.info(f"Processing {len(transactions)} frozen transactions for week {current.week}") # Resolve contested transactions - winning_move_ids, losing_move_ids = await resolve_contested_transactions( + winning_move_ids, losing_move_ids, conflict_resolutions = await resolve_contested_transactions( transactions, current.season ) + # Build mapping from conflict player to winner for cancelled move tracking + conflict_player_to_winner: Dict[str, str] = {} + for conflict in conflict_resolutions: + conflict_player_to_winner[conflict.player_name.lower()] = conflict.winner.team_abbrev + + # Track cancelled moves for report + cancelled_moves_report: List[CancelledMove] = [] + # Cancel losing transactions via service for losing_move_id in losing_move_ids: try: @@ -452,6 +573,29 @@ class TransactionFreezeTask: await self._notify_gm_of_cancellation(first_move, team_for_notification) + # Find which player caused the conflict + contested_player = "" + lost_to = "" + for move in losing_moves: + player_key = move.player.name.lower() + if player_key in conflict_player_to_winner: + contested_player = move.player.name + lost_to = conflict_player_to_winner[player_key] + break + + # Build report entry + players = [ + (move.player.name, move.player.wara, move.oldteam.abbrev, move.newteam.abbrev) + for move in losing_moves + ] + cancelled_moves_report.append(CancelledMove( + move_id=losing_move_id, + team_abbrev=team_for_notification.abbrev, + players=players, + lost_to=lost_to, + contested_player=contested_player + )) + contested_players = [move.player.name for move in losing_moves] self.logger.info( f"Cancelled transaction {losing_move_id} due to contested players: " @@ -461,6 +605,9 @@ class TransactionFreezeTask: except Exception as e: self.logger.error(f"Error cancelling transaction {losing_move_id}: {e}") + # Track thawed moves for report + thawed_moves_report: List[ThawedMove] = [] + # Unfreeze winning transactions and post to log via service for winning_move_id in winning_move_ids: try: @@ -476,11 +623,53 @@ class TransactionFreezeTask: # Post to transaction log await self._post_transaction_to_log(winning_move_id, transactions) + # Build report entry + if winning_moves: + first_move = winning_moves[0] + # Extract timestamp from moveid (format: Season-XXX-Week-XX-DD-HH:MM:SS) + try: + parts = winning_move_id.split('-') + submitted_at = parts[-1] if len(parts) >= 6 else "Unknown" + except Exception: + submitted_at = "Unknown" + + # Determine team abbrev + if first_move.newteam.abbrev.upper() != 'FA': + team_abbrev = first_move.newteam.abbrev + else: + team_abbrev = first_move.oldteam.abbrev + + players = [ + (move.player.name, move.player.wara, move.oldteam.abbrev, move.newteam.abbrev) + for move in winning_moves + ] + thawed_moves_report.append(ThawedMove( + move_id=winning_move_id, + team_abbrev=team_abbrev, + players=players, + submitted_at=submitted_at + )) + self.logger.info(f"Processed successful transaction {winning_move_id}") except Exception as e: self.logger.error(f"Error processing winning transaction {winning_move_id}: {e}") + # Generate and post thaw report + thaw_report = ThawReport( + week=current.week, + season=current.season, + timestamp=datetime.now(UTC), + total_moves=len(set(t.moveid for t in transactions)), + thawed_count=len(winning_move_ids), + cancelled_count=len(losing_move_ids), + conflict_count=len(conflict_resolutions), + conflicts=conflict_resolutions, + thawed_moves=thawed_moves_report, + cancelled_moves=cancelled_moves_report + ) + await self._post_thaw_report(thaw_report) + self.logger.info( f"Freeze processing complete: {len(winning_move_ids)} successful transactions, " f"{len(losing_move_ids)} cancelled transactions" @@ -770,6 +959,118 @@ class TransactionFreezeTask: except Exception as e: self.logger.error(f"Error notifying GM of cancellation: {e}") + async def _post_thaw_report(self, report: ThawReport): + """ + Post the thaw report to the admin channel for league admins. + + Args: + report: ThawReport containing all thaw processing details + """ + try: + config = get_config() + guild = self.bot.get_guild(config.guild_id) + if not guild: + self.logger.warning("Could not find guild for thaw report") + return + + # Try to find admin channel (admin, bot-admin, or bot-logs) + admin_channel = None + for channel_name in ['bot-admin', 'admin', 'bot-logs']: + admin_channel = discord.utils.get(guild.text_channels, name=channel_name) + if admin_channel: + break + + if not admin_channel: + self.logger.warning("Could not find admin channel for thaw report") + return + + # Build the report content + report_lines = [] + + # Header with summary + timestamp_str = report.timestamp.strftime('%B %d, %Y %H:%M UTC') + report_lines.append(f"# Transaction Thaw Report") + report_lines.append(f"**Week {report.week}** | **Season {report.season}** | {timestamp_str}") + report_lines.append(f"**Total:** {report.total_moves} moves | **Thawed:** {report.thawed_count} | **Cancelled:** {report.cancelled_count} | **Conflicts:** {report.conflict_count}") + report_lines.append("") + + # Conflict Resolution section (if any) + if report.conflicts: + report_lines.append("## Conflict Resolution") + for conflict in report.conflicts: + report_lines.append(f"**{conflict.player_name}** (sWAR: {conflict.player_swar:.1f})") + contenders_str = " vs ".join([ + f"{c.team_abbrev} ({c.wins}-{c.losses})" + for c in conflict.contenders + ]) + report_lines.append(f"- Contested by: {contenders_str}") + report_lines.append(f"- **Awarded to: {conflict.winner.team_abbrev}** (worst record wins)") + report_lines.append("") + + # Thawed Moves section + report_lines.append("## Thawed Moves") + if report.thawed_moves: + for move in report.thawed_moves: + report_lines.append(f"**{move.move_id}** | {move.team_abbrev}") + for player_name, swar, old_team, new_team in move.players: + report_lines.append(f" - {player_name} ({swar:.1f}): {old_team} → {new_team}") + else: + report_lines.append("*No moves thawed*") + report_lines.append("") + + # Cancelled Moves section + report_lines.append("## Cancelled Moves") + if report.cancelled_moves: + for move in report.cancelled_moves: + lost_info = f" (lost {move.contested_player} to {move.lost_to})" if move.lost_to else "" + report_lines.append(f"**{move.move_id}** | {move.team_abbrev}{lost_info}") + for player_name, swar, old_team, new_team in move.players: + report_lines.append(f" - ❌ {player_name} ({swar:.1f}): {old_team} → {new_team}") + else: + report_lines.append("*No moves cancelled*") + + # Combine into content + report_content = "\n".join(report_lines) + + # Discord has a 2000 character limit, so we may need to split + if len(report_content) <= 2000: + await admin_channel.send(report_content) + else: + # Split into multiple messages + await self._send_long_report(admin_channel, report_lines) + + self.logger.info(f"Thaw report posted to {admin_channel.name}") + + except Exception as e: + self.logger.error(f"Error posting thaw report: {e}", exc_info=True) + + async def _send_long_report(self, channel, report_lines: List[str]): + """ + Send a long report by splitting into multiple messages. + + Args: + channel: Discord channel to send to + report_lines: List of report lines to send + """ + current_chunk = [] + current_length = 0 + + for line in report_lines: + line_length = len(line) + 1 # +1 for newline + + if current_length + line_length > 1900: # Leave buffer for safety + # Send current chunk + await channel.send("\n".join(current_chunk)) + current_chunk = [line] + current_length = line_length + else: + current_chunk.append(line) + current_length += line_length + + # Send remaining chunk + if current_chunk: + await channel.send("\n".join(current_chunk)) + async def _send_owner_notification(self, message: str): """ Send error notification to bot owner. diff --git a/tests/test_tasks_transaction_freeze.py b/tests/test_tasks_transaction_freeze.py index 1fbc0d6..f3fc032 100644 --- a/tests/test_tasks_transaction_freeze.py +++ b/tests/test_tasks_transaction_freeze.py @@ -343,7 +343,7 @@ class TestResolveContestedTransactions: with patch('tasks.transaction_freeze.standings_service') as mock_standings: mock_standings.get_team_standings = AsyncMock(return_value=None) - winning_ids, losing_ids = await resolve_contested_transactions(transactions, 12) + winning_ids, losing_ids, conflicts = await resolve_contested_transactions(transactions, 12) # Single transaction should win automatically assert sample_transaction.moveid in winning_ids @@ -369,7 +369,7 @@ class TestResolveContestedTransactions: # Mock random for deterministic testing with patch('tasks.transaction_freeze.random.randint', return_value=50000): - winning_ids, losing_ids = await resolve_contested_transactions( + winning_ids, losing_ids, conflicts = await resolve_contested_transactions( contested_transactions, 12 ) @@ -439,7 +439,7 @@ class TestResolveContestedTransactions: # Mock random for deterministic testing with patch('tasks.transaction_freeze.random.randint', return_value=50000): - winning_ids, losing_ids = await resolve_contested_transactions(transactions, 12) + winning_ids, losing_ids, conflicts = await resolve_contested_transactions(transactions, 12) # Should have called with "WV" (stripped "MiL" suffix) # Will be called twice (once for WVMiL, once for NY) @@ -477,7 +477,7 @@ class TestResolveContestedTransactions: transactions = [drop_tx] - winning_ids, losing_ids = await resolve_contested_transactions(transactions, 12) + winning_ids, losing_ids, conflicts = await resolve_contested_transactions(transactions, 12) # FA drops are not winners or losers (they're not acquisitions) assert len(winning_ids) == 0 @@ -492,7 +492,7 @@ class TestResolveContestedTransactions: # Mock random for deterministic testing with patch('tasks.transaction_freeze.random.randint', return_value=50000): - winning_ids, losing_ids = await resolve_contested_transactions( + winning_ids, losing_ids, conflicts = await resolve_contested_transactions( contested_transactions, 12 ) @@ -564,7 +564,7 @@ class TestResolveContestedTransactions: mock_standings.get_team_standings = AsyncMock(side_effect=get_standings) with patch('tasks.transaction_freeze.random.randint', return_value=50000): - winning_ids, losing_ids = await resolve_contested_transactions(transactions, 12) + winning_ids, losing_ids, conflicts = await resolve_contested_transactions(transactions, 12) # Only one winner assert len(winning_ids) == 1 @@ -809,9 +809,11 @@ class TestProcessFrozenTransactions: mock_tx_service.unfreeze_transaction = AsyncMock(return_value=True) with patch('tasks.transaction_freeze.resolve_contested_transactions') as mock_resolve: - mock_resolve.return_value = ([sample_transaction.moveid], []) + # Returns (winning_ids, losing_ids, conflict_resolutions) + mock_resolve.return_value = ([sample_transaction.moveid], [], []) task._post_transaction_to_log = AsyncMock() + task._post_thaw_report = AsyncMock() await task._process_frozen_transactions(frozen_state) @@ -844,11 +846,12 @@ class TestProcessFrozenTransactions: mock_tx_service.unfreeze_transaction = AsyncMock(return_value=True) with patch('tasks.transaction_freeze.resolve_contested_transactions') as mock_resolve: - # tx1 wins, tx2 loses - mock_resolve.return_value = ([tx1.moveid], [tx2.moveid]) + # tx1 wins, tx2 loses - returns (winning_ids, losing_ids, conflict_resolutions) + mock_resolve.return_value = ([tx1.moveid], [tx2.moveid], []) task._post_transaction_to_log = AsyncMock() task._notify_gm_of_cancellation = AsyncMock() + task._post_thaw_report = AsyncMock() await task._process_frozen_transactions(frozen_state) @@ -870,9 +873,15 @@ class TestProcessFrozenTransactions: with patch('tasks.transaction_freeze.transaction_service') as mock_tx_service: mock_tx_service.get_frozen_transactions_by_week = AsyncMock(return_value=None) + # Mock the thaw report posting (empty report is still posted for visibility) + task._post_thaw_report = AsyncMock() + # Should not raise error await task._process_frozen_transactions(frozen_state) + # Verify empty report was posted + task._post_thaw_report.assert_called_once() + @pytest.mark.asyncio async def test_process_frozen_transaction_error_recovery( self, @@ -892,9 +901,11 @@ class TestProcessFrozenTransactions: mock_tx_service.unfreeze_transaction = AsyncMock(return_value=False) with patch('tasks.transaction_freeze.resolve_contested_transactions') as mock_resolve: - mock_resolve.return_value = ([sample_transaction.moveid], []) + # Returns (winning_ids, losing_ids, conflict_resolutions) + mock_resolve.return_value = ([sample_transaction.moveid], [], []) task._post_transaction_to_log = AsyncMock() + task._post_thaw_report = AsyncMock() # Should not raise error await task._process_frozen_transactions(frozen_state) diff --git a/tests/test_views_transaction_embed.py b/tests/test_views_transaction_embed.py index a03dcce..b868862 100644 --- a/tests/test_views_transaction_embed.py +++ b/tests/test_views_transaction_embed.py @@ -255,6 +255,7 @@ class TestSubmitConfirmationModal: with patch('services.league_service.league_service') as mock_league_service: mock_current_state = MagicMock() mock_current_state.week = 10 + mock_current_state.freeze = True # Simulate Monday-Friday freeze period mock_league_service.get_current_state = AsyncMock(return_value=mock_current_state) modal.builder.submit_transaction = AsyncMock(return_value=[mock_transaction]) @@ -278,7 +279,7 @@ class TestSubmitConfirmationModal: # Should submit transaction for next week modal.builder.submit_transaction.assert_called_once_with(week=11) - # Should mark transaction as frozen + # Should mark transaction as frozen (based on current_state.freeze) assert mock_transaction.frozen is True # Should POST to database @@ -293,6 +294,45 @@ class TestSubmitConfirmationModal: assert "Transaction Submitted Successfully" in call_args[0][0] assert mock_transaction.moveid in call_args[0][0] + @pytest.mark.asyncio + async def test_modal_submit_during_thaw_period(self, mock_builder, mock_interaction): + """Test modal submission during thaw period (Saturday-Sunday) sets frozen=False. + + When freeze=False (Saturday-Sunday), transactions should NOT be frozen + because they should execute immediately on Monday morning without waiting + for the Saturday conflict resolution process. + """ + modal = SubmitConfirmationModal(mock_builder) + # Mock the TextInput values + modal.confirmation = MagicMock() + modal.confirmation.value = 'CONFIRM' + + mock_transaction = MagicMock() + mock_transaction.moveid = 'Season-012-Week-11-123456789' + mock_transaction.week = 11 + mock_transaction.frozen = True # Will be set to False during thaw + + with patch('services.league_service.league_service') as mock_league_service: + mock_current_state = MagicMock() + mock_current_state.week = 10 + mock_current_state.freeze = False # Simulate Saturday-Sunday thaw period + mock_league_service.get_current_state = AsyncMock(return_value=mock_current_state) + + modal.builder.submit_transaction = AsyncMock(return_value=[mock_transaction]) + + with patch('services.transaction_service.transaction_service') as mock_transaction_service: + # Mock the create_transaction_batch call + mock_transaction_service.create_transaction_batch = AsyncMock(return_value=[mock_transaction]) + + with patch('utils.transaction_logging.post_transaction_to_log') as mock_post_log: + mock_post_log.return_value = AsyncMock() + + with patch('services.transaction_builder.clear_transaction_builder') as mock_clear: + await modal.on_submit(mock_interaction) + + # Should mark transaction as NOT frozen (thaw period) + assert mock_transaction.frozen is False + @pytest.mark.asyncio async def test_modal_submit_no_current_state(self, mock_builder, mock_interaction): """Test modal submission when current state unavailable.""" diff --git a/views/transaction_embed.py b/views/transaction_embed.py index 0d78703..2c63e46 100644 --- a/views/transaction_embed.py +++ b/views/transaction_embed.py @@ -240,9 +240,11 @@ class SubmitConfirmationModal(discord.ui.Modal): # Submit the transaction for NEXT week transactions = await self.builder.submit_transaction(week=current_state.week + 1) - # Mark transactions as frozen for weekly processing + # Set frozen flag based on current freeze state: + # - During freeze (Mon-Fri): frozen=True -> wait for Saturday conflict resolution + # - During thaw (Sat-Sun): frozen=False -> execute next Monday immediately for txn in transactions: - txn.frozen = True + txn.frozen = current_state.freeze # POST transactions to database created_transactions = await transaction_service.create_transaction_batch(transactions)