Merge pull request #8 from calcorum/bug-transactions

Bug transactions

commands/transactions/dropadd.py

@@ -168,32 +168,60 @@ class DropAddCommands(commands.Cog):
             
             # Determine player's current roster status by checking actual roster data
             # Note: Minor League players have different team_id than Major League team
-            self.logger.debug(f"Player {player.name} team_id: {player.team_id}, Builder team_id: {builder.team.id}")
+            self.logger.debug(f"🔍 DIAGNOSTIC: Player {player.name} (ID={player.id}) team_id: {player.team_id}, Builder team_id: {builder.team.id}")
+            if player.team:
+                self.logger.debug(f"🔍 DIAGNOSTIC: Player team abbrev: {player.team.abbrev}")
 
             await builder.load_roster_data()
             if builder._current_roster:
+                # Log roster composition for diagnostics
+                ml_count = len(builder._current_roster.active_players)
+                mil_count = len(builder._current_roster.minor_league_players)
+                il_count = len(builder._current_roster.il_players)
+
+                self.logger.debug(f"🔍 DIAGNOSTIC: Roster loaded for {builder.team.abbrev}: "
+                               f"ML={ml_count}, MiL={mil_count}, IL={il_count}")
+
+                # Log ALL player IDs in each roster section
+                ml_ids = [p.id for p in builder._current_roster.active_players]
+                mil_ids = [p.id for p in builder._current_roster.minor_league_players]
+                il_ids = [p.id for p in builder._current_roster.il_players]
+
+                self.logger.debug(f"🔍 DIAGNOSTIC: ML player IDs: {ml_ids}")
+                self.logger.debug(f"🔍 DIAGNOSTIC: MiL player IDs: {mil_ids}")
+                self.logger.debug(f"🔍 DIAGNOSTIC: IL player IDs: {il_ids}")
+                self.logger.debug(f"🔍 DIAGNOSTIC: Searching for player ID: {player.id}")
+
                 # Check which roster section the player is on (regardless of team_id)
                 player_on_active = any(p.id == player.id for p in builder._current_roster.active_players)
                 player_on_minor = any(p.id == player.id for p in builder._current_roster.minor_league_players)
                 player_on_il = any(p.id == player.id for p in builder._current_roster.il_players)
 
+                self.logger.debug(f"🔍 DIAGNOSTIC: Player {player.name} found - ML:{player_on_active}, MiL:{player_on_minor}, IL:{player_on_il}")
+
                 if player_on_active:
                     from_roster = RosterType.MAJOR_LEAGUE
-                    self.logger.debug(f"Player {player.name} found on active roster (Major League)")
+                    self.logger.debug(f"✅ Player {player.name} found on active roster (Major League)")
                 elif player_on_minor:
                     from_roster = RosterType.MINOR_LEAGUE
-                    self.logger.debug(f"Player {player.name} found on minor league roster")
+                    self.logger.debug(f"✅ Player {player.name} found on minor league roster")
                 elif player_on_il:
                     from_roster = RosterType.INJURED_LIST
-                    self.logger.debug(f"Player {player.name} found on injured list")
+                    self.logger.debug(f"✅ Player {player.name} found on injured list")
                 else:
                     # Player not found on user's roster - they're from another team or free agency
                     from_roster = RosterType.FREE_AGENCY
-                    self.logger.debug(f"Player {player.name} not found on user's roster, treating as free agency")
+                    self.logger.warning(f"⚠️  Player {player.name} (ID={player.id}) not found on user's roster, treating as free agency")
+
+                    # Additional diagnostic: Check if player's team suggests they should be on roster
+                    if player.team and builder.team.is_same_organization(player.team):
+                        self.logger.error(f"❌ BUG DETECTED: Player {player.name} belongs to {player.team.abbrev} "
+                                        f"(same organization as {builder.team.abbrev}) but not found in roster lists!")
+                        self.logger.error(f"❌ Player team_id={player.team_id}, roster team_id={builder._current_roster.team_id}")
             else:
                 # Couldn't load roster data, assume free agency as safest fallback
                 from_roster = RosterType.FREE_AGENCY
-                self.logger.warning(f"Could not load roster data, assuming {player.name} is free agency")
+                self.logger.error(f"❌ Could not load roster data, assuming {player.name} is free agency")
             
             # Create move
             move = TransactionMove(

commands/transactions/ilmove.py

@@ -173,32 +173,61 @@ class ILMoveCommands(commands.Cog):
 
             # Determine player's current roster status by checking actual roster data
             # Note: Minor League players have different team_id than Major League team
-            self.logger.debug(f"Player {player.name} team_id: {player.team_id}, Builder team_id: {builder.team.id}")
+            self.logger.debug(f"🔍 DIAGNOSTIC: Player {player.name} (ID={player.id}) team_id: {player.team_id}, Builder team_id: {builder.team.id}")
+            if player.team:
+                self.logger.debug(f"🔍 DIAGNOSTIC: Player team abbrev: {player.team.abbrev}")
 
             await builder.load_roster_data()
             if builder._current_roster:
+                # Log roster composition for diagnostics
+                ml_count = len(builder._current_roster.active_players)
+                mil_count = len(builder._current_roster.minor_league_players)
+                il_count = len(builder._current_roster.il_players)
+
+                self.logger.debug(f"🔍 DIAGNOSTIC: Roster loaded for {builder.team.abbrev}: "
+                               f"ML={ml_count}, MiL={mil_count}, IL={il_count}")
+
+                # Log ALL player IDs in each roster section
+                ml_ids = [p.id for p in builder._current_roster.active_players]
+                mil_ids = [p.id for p in builder._current_roster.minor_league_players]
+                il_ids = [p.id for p in builder._current_roster.il_players]
+
+                self.logger.debug(f"🔍 DIAGNOSTIC: ML player IDs: {ml_ids}")
+                self.logger.debug(f"🔍 DIAGNOSTIC: MiL player IDs: {mil_ids}")
+                self.logger.debug(f"🔍 DIAGNOSTIC: IL player IDs: {il_ids}")
+                self.logger.debug(f"🔍 DIAGNOSTIC: Searching for player ID: {player.id}")
+
                 # Check which roster section the player is on (regardless of team_id)
                 player_on_active = any(p.id == player.id for p in builder._current_roster.active_players)
                 player_on_minor = any(p.id == player.id for p in builder._current_roster.minor_league_players)
                 player_on_il = any(p.id == player.id for p in builder._current_roster.il_players)
 
+                self.logger.debug(f"🔍 DIAGNOSTIC: Player {player.name} found - ML:{player_on_active}, MiL:{player_on_minor}, IL:{player_on_il}")
+
                 if player_on_active:
                     from_roster = RosterType.MAJOR_LEAGUE
-                    self.logger.debug(f"Player {player.name} found on active roster (Major League)")
+                    self.logger.debug(f"✅ Player {player.name} found on active roster (Major League)")
                 elif player_on_minor:
                     from_roster = RosterType.MINOR_LEAGUE
-                    self.logger.debug(f"Player {player.name} found on minor league roster")
+                    self.logger.debug(f"✅ Player {player.name} found on minor league roster")
                 elif player_on_il:
                     from_roster = RosterType.INJURED_LIST
-                    self.logger.debug(f"Player {player.name} found on injured list")
+                    self.logger.debug(f"✅ Player {player.name} found on injured list")
                 else:
                     # Player not found on user's roster - cannot move with /ilmove
                     from_roster = None
-                    self.logger.warning(f"Player {player.name} not found on {builder.team.abbrev} roster")
+                    self.logger.warning(f"⚠️  Player {player.name} (ID={player.id}) not found on {builder.team.abbrev} roster")
+
+                    # Additional diagnostic: Check if player's team suggests they should be on roster
+                    if player.team and builder.team.is_same_organization(player.team):
+                        self.logger.error(f"❌ BUG DETECTED: Player {player.name} belongs to {player.team.abbrev} "
+                                        f"(same organization as {builder.team.abbrev}) but not found in roster lists!")
+                        self.logger.error(f"❌ Player team_id={player.team_id}, roster team_id={builder._current_roster.team_id}")
+
                     return False, f"{player.name} is not on your roster (use /dropadd for FA signings)"
             else:
                 # Couldn't load roster data
-                self.logger.error(f"Could not load roster data for {builder.team.abbrev}")
+                self.logger.error(f"❌ Could not load roster data for {builder.team.abbrev}")
                 return False, "Could not load roster data. Please try again."
 
             if from_roster is None:

services/transaction_builder.py

@@ -338,21 +338,29 @@ class TransactionBuilder:
                     # Note: IL players don't count toward roster limits, so no changes needed
 
         for move in self.moves:
+            # Log move being processed for diagnostics
+            logger.debug(f"🔍 VALIDATION: Processing move - {move.player.name} (ID={move.player.id})")
+            logger.debug(f"🔍 VALIDATION: from_roster={move.from_roster.value}, to_roster={move.to_roster.value}")
+
             # Calculate roster changes based on from/to locations
             if move.from_roster == RosterType.MAJOR_LEAGUE:
                 ml_changes -= 1
                 ml_swar_changes -= move.player.wara
+                logger.debug(f"🔍 VALIDATION: ML decrement - ml_changes now {ml_changes}")
             elif move.from_roster == RosterType.MINOR_LEAGUE:
                 mil_changes -= 1
                 mil_swar_changes -= move.player.wara
+                logger.debug(f"🔍 VALIDATION: MiL decrement - mil_changes now {mil_changes}")
             # Note: INJURED_LIST and FREE_AGENCY don't count toward ML roster limit
 
             if move.to_roster == RosterType.MAJOR_LEAGUE:
                 ml_changes += 1
                 ml_swar_changes += move.player.wara
+                logger.debug(f"🔍 VALIDATION: ML increment - ml_changes now {ml_changes}")
             elif move.to_roster == RosterType.MINOR_LEAGUE:
                 mil_changes += 1
                 mil_swar_changes += move.player.wara
+                logger.debug(f"🔍 VALIDATION: MiL increment - mil_changes now {mil_changes}")
             # Note: INJURED_LIST and FREE_AGENCY don't count toward ML roster limit
         
         # Calculate projected roster sizes and sWAR
@@ -360,10 +368,16 @@ class TransactionBuilder:
         current_ml_size = len(self._current_roster.active_players)
         current_mil_size = len(self._current_roster.minor_league_players)
 
+        logger.debug(f"🔍 VALIDATION: Current roster - ML:{current_ml_size}, MiL:{current_mil_size}")
+        logger.debug(f"🔍 VALIDATION: Changes calculated - ml_changes:{ml_changes}, mil_changes:{mil_changes}")
+
         projected_ml_size = current_ml_size + ml_changes
         projected_mil_size = current_mil_size + mil_changes
         projected_ml_swar = current_ml_swar + ml_swar_changes
         projected_mil_swar = current_mil_swar + mil_swar_changes
+
+        logger.debug(f"🔍 VALIDATION: Projected roster - ML:{projected_ml_size}, MiL:{projected_mil_size}")
+        logger.debug(f"🔍 VALIDATION: Projected sWAR - ML:{projected_ml_swar:.2f}, MiL:{projected_mil_swar:.2f}")
         
         # Get current week to determine roster limits
         try:
@@ -421,31 +435,40 @@ class TransactionBuilder:
             pre_existing_transaction_count=pre_existing_count
         )
     
-    async def submit_transaction(self, week: int) -> List[Transaction]:
+    async def submit_transaction(self, week: int, check_existing_transactions: bool = True) -> List[Transaction]:
         """
         Submit the transaction by creating individual Transaction models.
-        
+
         Args:
             week: Week the transaction is effective for
-            
+            check_existing_transactions: Whether to include pre-existing transactions in validation.
+                Set to True for /dropadd (scheduled moves - need to check against other scheduled moves).
+                Set to False for /ilmove (immediate moves - already in database, don't double-count).
+
         Returns:
             List of created Transaction objects
         """
         if not self.moves:
             raise ValueError("Cannot submit empty transaction")
-        
+
-        validation = await self.validate_transaction(next_week=week)
+        # For immediate moves (/ilmove), don't check pre-existing transactions
+        if check_existing_transactions:
+            validation = await self.validate_transaction(next_week=week)
+        else:
+            validation = await self.validate_transaction()
+
         if not validation.is_legal:
             raise ValueError(f"Cannot submit illegal transaction: {', '.join(validation.errors)}")
         
         transactions = []
         move_id = f"Season-{self.season:03d}-Week-{week:02d}-{int(self.created_at.timestamp())}"
         
-        # Create FA team for drops
+        # Create FA team for drops using config value
+        config = get_config()
         fa_team = Team(
-            id=503,  # Standard FA team ID
+            id=config.free_agent_team_id,  # Correct FA team ID from config (498)
             abbrev="FA",
-            sname="Free Agents", 
+            sname="Free Agents",
             lname="Free Agency",
             season=self.season
         ) # type: ignore

utils/transaction_logging.py

@@ -0,0 +1,144 @@
+"""
+Transaction Logging Utility
+
+Provides centralized function for posting transaction notifications
+to the #transaction-log channel.
+"""
+from typing import List, Optional
+import discord
+
+from config import get_config
+from models.transaction import Transaction
+from models.team import Team
+from views.embeds import EmbedTemplate, EmbedColors
+from utils.logging import get_contextual_logger
+
+logger = get_contextual_logger(f'{__name__}')
+
+
+async def post_transaction_to_log(
+    bot: discord.Client,
+    transactions: List[Transaction],
+    team: Optional[Team] = None
+) -> bool:
+    """
+    Post a transaction to the #transaction-log channel.
+
+    Args:
+        bot: Discord bot instance
+        transactions: List of Transaction objects to post
+        team: Optional team override (if None, determined from transactions)
+
+    Returns:
+        True if posted successfully, False otherwise
+    """
+    try:
+        if not transactions:
+            logger.warning("No transactions provided to post_transaction_to_log")
+            return False
+
+        # Get guild and channel
+        config = get_config()
+        guild = bot.get_guild(config.guild_id)
+        if not guild:
+            logger.warning(f"Could not find guild {config.guild_id}")
+            return False
+
+        channel = discord.utils.get(guild.text_channels, name='transaction-log')
+        if not channel:
+            logger.warning("Could not find #transaction-log channel")
+            return False
+
+        # Determine the team for the embed (team making the moves)
+        if team is None:
+            team = await _determine_team_from_transactions(transactions)
+
+        # Build move string
+        move_string = ""
+        week_num = transactions[0].week
+        season = transactions[0].season
+
+        for txn in transactions:
+            # Format: PlayerName (sWAR) from OLDTEAM to NEWTEAM
+            move_string += (
+                f'**{txn.player.name}** ({txn.player.wara:.2f}) '
+                f'from {txn.oldteam.abbrev} to {txn.newteam.abbrev}\n'
+            )
+
+        # Create embed matching legacy format
+        embed = EmbedTemplate.create_base_embed(
+            title=f'Week {week_num} Transaction',
+            description=team.sname if hasattr(team, 'sname') else team.lname,
+            color=EmbedColors.INFO
+        )
+
+        # Set team color if available
+        if hasattr(team, 'color') and team.color:
+            try:
+                # Remove # if present and convert to int
+                color_hex = team.color.replace('#', '')
+                embed.color = discord.Color(int(color_hex, 16))
+            except (ValueError, AttributeError):
+                pass  # Use default color on error
+
+        # Set team thumbnail if available
+        if hasattr(team, 'thumbnail') and team.thumbnail:
+            embed.set_thumbnail(url=team.thumbnail)
+
+        # Add player moves field
+        embed.add_field(name='Player Moves', value=move_string, inline=False)
+
+        # Add footer with SBA branding using current season from transaction
+        embed.set_footer(
+            text=f"SBa Season {season}",
+            icon_url="https://sombaseball.ddns.net/static/images/sba-logo.png"
+        )
+
+        # Post to channel
+        await channel.send(embed=embed)
+        logger.info(f"Transaction posted to log: {transactions[0].moveid}, {len(transactions)} moves")
+        return True
+
+    except Exception as e:
+        logger.error(f"Error posting transaction to log: {e}")
+        return False
+
+
+async def _determine_team_from_transactions(transactions: List[Transaction]) -> Team:
+    """
+    Determine which team to display for the transaction embed.
+
+    Uses the major league affiliate of the team involved in the transaction
+    to ensure consistent branding (no MiL or IL team logos).
+
+    Logic:
+    - Use newteam's ML affiliate if it's not FA
+    - Otherwise use oldteam's ML affiliate if it's not FA
+    - Otherwise default to newteam
+
+    Args:
+        transactions: List of transactions
+
+    Returns:
+        Team to display in the embed (always Major League team)
+    """
+    first_move = transactions[0]
+
+    # Check newteam first
+    if first_move.newteam.abbrev.upper() != 'FA':
+        try:
+            return await first_move.newteam.major_league_affiliate()
+        except Exception as e:
+            logger.warning(f"Could not get ML affiliate for {first_move.newteam.abbrev}: {e}")
+            return first_move.newteam
+
+    # Check oldteam
+    if first_move.oldteam.abbrev.upper() != 'FA':
+        try:
+            return await first_move.oldteam.major_league_affiliate()
+        except Exception as e:
+            logger.warning(f"Could not get ML affiliate for {first_move.oldteam.abbrev}: {e}")
+            return first_move.oldteam
+
+    # Default to newteam (both are FA)
+    return first_move.newteam

views/transaction_embed.py

@@ -9,6 +9,7 @@ from datetime import datetime
 
 from services.transaction_builder import TransactionBuilder, RosterValidationResult
 from views.embeds import EmbedColors, EmbedTemplate
+from utils.transaction_logging import post_transaction_to_log
 
 
 class TransactionEmbedView(discord.ui.View):
@@ -239,6 +240,10 @@ class SubmitConfirmationModal(discord.ui.Modal):
                 # Submit the transaction for NEXT week
                 transactions = await self.builder.submit_transaction(week=current_state.week + 1)
 
+                # Post to #transaction-log channel
+                bot = interaction.client
+                await post_transaction_to_log(bot, transactions, team=self.builder.team)
+
                 # Create success message
                 success_msg = f"✅ **Transaction Submitted Successfully!**\n\n"
                 success_msg += f"**Move ID:** `{transactions[0].moveid}`\n"
@@ -256,7 +261,11 @@ class SubmitConfirmationModal(discord.ui.Modal):
             elif self.submission_handler == "immediate":
                 # IMMEDIATE SUBMISSION (/ilmove behavior)
                 # Submit the transaction for THIS week
-                transactions = await self.builder.submit_transaction(week=current_state.week)
+                # Don't check existing transactions - they're already in DB and would cause double-counting
+                transactions = await self.builder.submit_transaction(
+                    week=current_state.week,
+                    check_existing_transactions=False
+                )
 
                 # POST transactions to database
                 created_transactions = await transaction_service.create_transaction_batch(transactions)
@@ -270,6 +279,10 @@ class SubmitConfirmationModal(discord.ui.Modal):
                     )
                     player_updates.append(updated_player)
 
+                # Post to #transaction-log channel
+                bot = interaction.client
+                await post_transaction_to_log(bot, created_transactions, team=self.builder.team)
+
                 # Create success message
                 success_msg = f"✅ **IL Move Executed Successfully!**\n\n"
                 success_msg += f"**Move ID:** `{created_transactions[0].moveid}`\n"