Compare commits

...

9 Commits

Author SHA1 Message Date
cal
c8cb80c5f3 Merge pull request 'feat: enforce FA lock deadline after week 14' (#122) from feature/fa-lock-enforcement into main
All checks were successful
Build Docker Image / build (push) Successful in 1m22s
Reviewed-on: #122
2026-03-31 21:20:41 +00:00
Cal Corum
6016afb999 feat: enforce FA lock deadline — block signing FA players after week 14
The fa_lock_week config existed but was never enforced. Now /dropadd blocks
adding players FROM Free Agency when current_week >= fa_lock_week (14).
Dropping players TO FA remains allowed after the deadline.

Also consolidates two league_service.get_current_state() calls into one
shared fetch at the top of add_move() to eliminate a redundant API round-trip.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-31 16:07:28 -05:00
cal
f95c857363 Merge pull request 'perf: eliminate redundant API calls in trade views (#94)' (#116) from ai/major-domo-v2#94 into main
Reviewed-on: #116
2026-03-31 19:54:18 +00:00
Cal Corum
174ce4474d fix: use per-user dict for _checked_teams to prevent race condition (#116)
Replace single `_checked_team: Optional[Team]` with `_checked_teams: dict[int, Team]`
keyed by `interaction.user.id`. The previous single field was shared across all
concurrent interaction tasks, allowing User B's interaction_check to overwrite
`_checked_team` before User A's button handler read it — crediting the wrong team.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-31 19:46:42 +00:00
Cal Corum
2091302b8a perf: eliminate redundant API calls in trade views (#94)
Closes #94

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-31 19:46:42 +00:00
cal
27a272b813 Merge pull request 'perf: eliminate redundant GET after create/update and parallelize stats (#95)' (#112) from ai/major-domo-v2#95 into main
Reviewed-on: #112
2026-03-31 19:46:19 +00:00
Cal Corum
95010bfd5d perf: eliminate redundant GET after create/update and parallelize stats (#95)
- custom_commands_service: return POST response directly from create_command()
  instead of a follow-up GET by_name
- custom_commands_service: return PUT response directly from update_command()
  instead of a follow-up GET by_name
- custom_commands_service: avoid GET after PUT in get_or_create_creator() by
  constructing updated creator from model_copy()
- custom_commands_service: return POST response directly from get_or_create_creator()
  creator creation instead of a follow-up GET
- custom_commands_service: parallelize all 9 sequential API calls in
  get_statistics() with asyncio.gather()
- help_commands_service: return POST response directly from create_help()
  instead of a follow-up GET by_name
- help_commands_service: return PUT response directly from update_help()
  instead of a follow-up GET by_name
- tests: update test_update_help_success to mock PUT returning dict data

Closes #95

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-31 19:45:44 +00:00
cal
deb40476a4 Merge pull request 'perf: parallelize N+1 player/creator lookups with asyncio.gather (#89)' (#118) from ai/major-domo-v2-89 into main
Reviewed-on: #118
2026-03-31 19:45:01 +00:00
Cal Corum
65d3099a7c perf: parallelize N+1 player/creator lookups with asyncio.gather (#89)
Closes #89

Replace sequential per-item await loops with asyncio.gather() to fetch
all results in parallel:

- decision_service.find_winning_losing_pitchers: gather wp, lp, sv,
  hold_ids, and bsv_ids (5-10 calls) in a single parallel batch
- custom_commands_service: parallelize get_creator_by_id() in
  get_popular_commands, get_commands_needing_warning, and
  get_commands_eligible_for_deletion using return_exceptions=True to
  preserve the existing BotException-skip / re-raise-other behavior

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-31 19:42:53 +00:00
7 changed files with 482 additions and 344 deletions

View File

@ -4,6 +4,7 @@ Custom Commands Service for Discord Bot v2.0
Modern async service layer for managing custom commands with full type safety.
"""
import asyncio
import math
from datetime import UTC, datetime, timedelta
from typing import Optional, List, Any, Tuple
@ -119,8 +120,8 @@ class CustomCommandsService(BaseService[CustomCommand]):
content_length=len(content),
)
# Return full command with creator info
return await self.get_command_by_name(name)
# Return command with creator info (use POST response directly)
return result.model_copy(update={"creator": creator})
async def get_command_by_name(self, name: str) -> CustomCommand:
"""
@ -217,7 +218,8 @@ class CustomCommandsService(BaseService[CustomCommand]):
new_content_length=len(new_content),
)
return await self.get_command_by_name(name)
# Return updated command with creator info (use PUT response directly)
return result.model_copy(update={"creator": command.creator})
async def delete_command(
self, name: str, deleter_discord_id: int, force: bool = False
@ -466,21 +468,28 @@ class CustomCommandsService(BaseService[CustomCommand]):
commands_data = await self.get_items_with_params(params)
creators = await asyncio.gather(
*[
self.get_creator_by_id(cmd_data.creator_id)
for cmd_data in commands_data
],
return_exceptions=True,
)
commands = []
for cmd_data in commands_data:
try:
creator = await self.get_creator_by_id(cmd_data.creator_id)
commands.append(CustomCommand(**cmd_data.model_dump(), creator=creator))
except BotException as e:
# Handle missing creator gracefully
for cmd_data, creator in zip(commands_data, creators):
if isinstance(creator, BotException):
self.logger.warning(
"Skipping popular command with missing creator",
command_id=cmd_data.id,
command_name=cmd_data.name,
creator_id=cmd_data.creator_id,
error=str(e),
error=str(creator),
)
continue
if isinstance(creator, BaseException):
raise creator
commands.append(CustomCommand(**cmd_data.model_dump(), creator=creator))
return commands
@ -536,7 +545,9 @@ class CustomCommandsService(BaseService[CustomCommand]):
# Update username if it changed
if creator.username != username or creator.display_name != display_name:
await self._update_creator_info(creator.id, username, display_name)
creator = await self.get_creator_by_discord_id(discord_id)
creator = creator.model_copy(
update={"username": username, "display_name": display_name}
)
return creator
except BotException:
# Creator doesn't exist, create new one
@ -557,7 +568,8 @@ class CustomCommandsService(BaseService[CustomCommand]):
if not result:
raise BotException("Failed to create command creator")
return await self.get_creator_by_discord_id(discord_id)
# Return created creator directly from POST response
return CustomCommandCreator(**result)
async def get_creator_by_discord_id(self, discord_id: int) -> CustomCommandCreator:
"""Get creator by Discord ID.
@ -610,31 +622,34 @@ class CustomCommandsService(BaseService[CustomCommand]):
async def get_statistics(self) -> CustomCommandStats:
"""Get comprehensive statistics about custom commands."""
# Get basic counts
total_commands = await self._get_search_count([])
active_commands = await self._get_search_count([("is_active", True)])
total_creators = await self._get_creator_count()
# Get total uses
all_commands = await self.get_items_with_params([("is_active", True)])
total_uses = sum(cmd.use_count for cmd in all_commands)
# Get most popular command
popular_commands = await self.get_popular_commands(limit=1)
most_popular = popular_commands[0] if popular_commands else None
# Get most active creator
most_active_creator = await self._get_most_active_creator()
# Get recent commands count
week_ago = datetime.now(UTC) - timedelta(days=7)
recent_count = await self._get_search_count(
[("created_at__gte", week_ago.isoformat()), ("is_active", True)]
(
total_commands,
active_commands,
total_creators,
all_commands,
popular_commands,
most_active_creator,
recent_count,
warning_count,
deletion_count,
) = await asyncio.gather(
self._get_search_count([]),
self._get_search_count([("is_active", True)]),
self._get_creator_count(),
self.get_items_with_params([("is_active", True)]),
self.get_popular_commands(limit=1),
self._get_most_active_creator(),
self._get_search_count(
[("created_at__gte", week_ago.isoformat()), ("is_active", True)]
),
self._get_commands_needing_warning_count(),
self._get_commands_eligible_for_deletion_count(),
)
# Get cleanup statistics
warning_count = await self._get_commands_needing_warning_count()
deletion_count = await self._get_commands_eligible_for_deletion_count()
total_uses = sum(cmd.use_count for cmd in all_commands)
most_popular = popular_commands[0] if popular_commands else None
return CustomCommandStats(
total_commands=total_commands,
@ -662,21 +677,28 @@ class CustomCommandsService(BaseService[CustomCommand]):
commands_data = await self.get_items_with_params(params)
creators = await asyncio.gather(
*[
self.get_creator_by_id(cmd_data.creator_id)
for cmd_data in commands_data
],
return_exceptions=True,
)
commands = []
for cmd_data in commands_data:
try:
creator = await self.get_creator_by_id(cmd_data.creator_id)
commands.append(CustomCommand(**cmd_data.model_dump(), creator=creator))
except BotException as e:
# Handle missing creator gracefully
for cmd_data, creator in zip(commands_data, creators):
if isinstance(creator, BotException):
self.logger.warning(
"Skipping command with missing creator",
command_id=cmd_data.id,
command_name=cmd_data.name,
creator_id=cmd_data.creator_id,
error=str(e),
error=str(creator),
)
continue
if isinstance(creator, BaseException):
raise creator
commands.append(CustomCommand(**cmd_data.model_dump(), creator=creator))
return commands
@ -688,21 +710,28 @@ class CustomCommandsService(BaseService[CustomCommand]):
commands_data = await self.get_items_with_params(params)
creators = await asyncio.gather(
*[
self.get_creator_by_id(cmd_data.creator_id)
for cmd_data in commands_data
],
return_exceptions=True,
)
commands = []
for cmd_data in commands_data:
try:
creator = await self.get_creator_by_id(cmd_data.creator_id)
commands.append(CustomCommand(**cmd_data.model_dump(), creator=creator))
except BotException as e:
# Handle missing creator gracefully
for cmd_data, creator in zip(commands_data, creators):
if isinstance(creator, BotException):
self.logger.warning(
"Skipping command with missing creator",
command_id=cmd_data.id,
command_name=cmd_data.name,
creator_id=cmd_data.creator_id,
error=str(e),
error=str(creator),
)
continue
if isinstance(creator, BaseException):
raise creator
commands.append(CustomCommand(**cmd_data.model_dump(), creator=creator))
return commands

View File

@ -4,6 +4,7 @@ Decision Service
Manages pitching decision operations for game submission.
"""
import asyncio
from typing import List, Dict, Any, Optional, Tuple
from utils.logging import get_contextual_logger
@ -124,22 +125,19 @@ class DecisionService:
if int(decision.get("b_save", 0)) == 1:
bsv_ids.append(pitcher_id)
# Second pass: Fetch Player objects
wp = await player_service.get_player(wp_id) if wp_id else None
lp = await player_service.get_player(lp_id) if lp_id else None
sv = await player_service.get_player(sv_id) if sv_id else None
# Second pass: Fetch all Player objects in parallel
# Order: [wp_id, lp_id, sv_id, *hold_ids, *bsv_ids]; None IDs resolve immediately
ordered_ids = [wp_id, lp_id, sv_id] + hold_ids + bsv_ids
results = await asyncio.gather(
*[
player_service.get_player(pid) if pid else asyncio.sleep(0, result=None)
for pid in ordered_ids
]
)
holders = []
for hold_id in hold_ids:
holder = await player_service.get_player(hold_id)
if holder:
holders.append(holder)
blown_saves = []
for bsv_id in bsv_ids:
bsv = await player_service.get_player(bsv_id)
if bsv:
blown_saves.append(bsv)
wp, lp, sv = results[0], results[1], results[2]
holders = [p for p in results[3 : 3 + len(hold_ids)] if p]
blown_saves = [p for p in results[3 + len(hold_ids) :] if p]
return wp, lp, sv, holders, blown_saves

View File

@ -5,6 +5,7 @@ Modern async service layer for managing help commands with full type safety.
Allows admins and help editors to create custom help topics for league documentation,
resources, FAQs, links, and guides.
"""
from typing import Optional, List
from utils.logging import get_contextual_logger
@ -12,7 +13,7 @@ from models.help_command import (
HelpCommand,
HelpCommandSearchFilters,
HelpCommandSearchResult,
HelpCommandStats
HelpCommandStats,
)
from services.base_service import BaseService
from exceptions import BotException
@ -20,16 +21,19 @@ from exceptions import BotException
class HelpCommandNotFoundError(BotException):
"""Raised when a help command is not found."""
pass
class HelpCommandExistsError(BotException):
"""Raised when trying to create a help command that already exists."""
pass
class HelpCommandPermissionError(BotException):
"""Raised when user lacks permission for help command operation."""
pass
@ -37,8 +41,8 @@ class HelpCommandsService(BaseService[HelpCommand]):
"""Service for managing help commands."""
def __init__(self):
super().__init__(HelpCommand, 'help_commands')
self.logger = get_contextual_logger(f'{__name__}.HelpCommandsService')
super().__init__(HelpCommand, "help_commands")
self.logger = get_contextual_logger(f"{__name__}.HelpCommandsService")
self.logger.info("HelpCommandsService initialized")
# === Command CRUD Operations ===
@ -50,7 +54,7 @@ class HelpCommandsService(BaseService[HelpCommand]):
content: str,
creator_discord_id: str,
category: Optional[str] = None,
display_order: int = 0
display_order: int = 0,
) -> HelpCommand:
"""
Create a new help command.
@ -80,14 +84,16 @@ class HelpCommandsService(BaseService[HelpCommand]):
# Create help command data
help_data = {
'name': name.lower().strip(),
'title': title.strip(),
'content': content.strip(),
'category': category.lower().strip() if category else None,
'created_by_discord_id': str(creator_discord_id), # Convert to string for safe storage
'display_order': display_order,
'is_active': True,
'view_count': 0
"name": name.lower().strip(),
"title": title.strip(),
"content": content.strip(),
"category": category.lower().strip() if category else None,
"created_by_discord_id": str(
creator_discord_id
), # Convert to string for safe storage
"display_order": display_order,
"is_active": True,
"view_count": 0,
}
# Create via API
@ -95,18 +101,18 @@ class HelpCommandsService(BaseService[HelpCommand]):
if not result:
raise BotException("Failed to create help command")
self.logger.info("Help command created",
help_name=name,
creator_id=creator_discord_id,
category=category)
self.logger.info(
"Help command created",
help_name=name,
creator_id=creator_discord_id,
category=category,
)
# Return full help command
return await self.get_help_by_name(name)
# Return help command directly from POST response
return result
async def get_help_by_name(
self,
name: str,
include_inactive: bool = False
self, name: str, include_inactive: bool = False
) -> HelpCommand:
"""
Get a help command by name.
@ -126,8 +132,12 @@ class HelpCommandsService(BaseService[HelpCommand]):
try:
# Use the dedicated by_name endpoint for exact lookup
client = await self.get_client()
params = [('include_inactive', include_inactive)] if include_inactive else []
data = await client.get(f'help_commands/by_name/{normalized_name}', params=params)
params = (
[("include_inactive", include_inactive)] if include_inactive else []
)
data = await client.get(
f"help_commands/by_name/{normalized_name}", params=params
)
if not data:
raise HelpCommandNotFoundError(f"Help topic '{name}' not found")
@ -139,9 +149,9 @@ class HelpCommandsService(BaseService[HelpCommand]):
if "404" in str(e) or "not found" in str(e).lower():
raise HelpCommandNotFoundError(f"Help topic '{name}' not found")
else:
self.logger.error("Failed to get help command by name",
help_name=name,
error=e)
self.logger.error(
"Failed to get help command by name", help_name=name, error=e
)
raise BotException(f"Failed to retrieve help topic '{name}': {e}")
async def update_help(
@ -151,7 +161,7 @@ class HelpCommandsService(BaseService[HelpCommand]):
new_content: Optional[str] = None,
updater_discord_id: Optional[str] = None,
new_category: Optional[str] = None,
new_display_order: Optional[int] = None
new_display_order: Optional[int] = None,
) -> HelpCommand:
"""
Update an existing help command.
@ -176,35 +186,42 @@ class HelpCommandsService(BaseService[HelpCommand]):
update_data = {}
if new_title is not None:
update_data['title'] = new_title.strip()
update_data["title"] = new_title.strip()
if new_content is not None:
update_data['content'] = new_content.strip()
update_data["content"] = new_content.strip()
if new_category is not None:
update_data['category'] = new_category.lower().strip() if new_category else None
update_data["category"] = (
new_category.lower().strip() if new_category else None
)
if new_display_order is not None:
update_data['display_order'] = new_display_order
update_data["display_order"] = new_display_order
if updater_discord_id is not None:
update_data['last_modified_by'] = str(updater_discord_id) # Convert to string for safe storage
update_data["last_modified_by"] = str(
updater_discord_id
) # Convert to string for safe storage
if not update_data:
raise BotException("No fields to update")
# Update via API
client = await self.get_client()
result = await client.put(f'help_commands/{help_cmd.id}', update_data)
result = await client.put(f"help_commands/{help_cmd.id}", update_data)
if not result:
raise BotException("Failed to update help command")
self.logger.info("Help command updated",
help_name=name,
updater_id=updater_discord_id,
fields_updated=list(update_data.keys()))
self.logger.info(
"Help command updated",
help_name=name,
updater_id=updater_discord_id,
fields_updated=list(update_data.keys()),
)
return await self.get_help_by_name(name)
# Return updated help command directly from PUT response
return self.model_class.from_api_data(result)
async def delete_help(self, name: str) -> bool:
"""
@ -223,11 +240,11 @@ class HelpCommandsService(BaseService[HelpCommand]):
# Soft delete via API
client = await self.get_client()
await client.delete(f'help_commands/{help_cmd.id}')
await client.delete(f"help_commands/{help_cmd.id}")
self.logger.info("Help command soft deleted",
help_name=name,
help_id=help_cmd.id)
self.logger.info(
"Help command soft deleted", help_name=name, help_id=help_cmd.id
)
return True
@ -252,13 +269,11 @@ class HelpCommandsService(BaseService[HelpCommand]):
# Restore via API
client = await self.get_client()
result = await client.patch(f'help_commands/{help_cmd.id}/restore')
result = await client.patch(f"help_commands/{help_cmd.id}/restore")
if not result:
raise BotException("Failed to restore help command")
self.logger.info("Help command restored",
help_name=name,
help_id=help_cmd.id)
self.logger.info("Help command restored", help_name=name, help_id=help_cmd.id)
return self.model_class.from_api_data(result)
@ -279,10 +294,9 @@ class HelpCommandsService(BaseService[HelpCommand]):
try:
client = await self.get_client()
await client.patch(f'help_commands/by_name/{normalized_name}/view')
await client.patch(f"help_commands/by_name/{normalized_name}/view")
self.logger.debug("Help command view count incremented",
help_name=name)
self.logger.debug("Help command view count incremented", help_name=name)
# Return updated command
return await self.get_help_by_name(name)
@ -291,16 +305,15 @@ class HelpCommandsService(BaseService[HelpCommand]):
if "404" in str(e) or "not found" in str(e).lower():
raise HelpCommandNotFoundError(f"Help topic '{name}' not found")
else:
self.logger.error("Failed to increment view count",
help_name=name,
error=e)
self.logger.error(
"Failed to increment view count", help_name=name, error=e
)
raise BotException(f"Failed to increment view count for '{name}': {e}")
# === Search and Listing ===
async def search_help_commands(
self,
filters: HelpCommandSearchFilters
self, filters: HelpCommandSearchFilters
) -> HelpCommandSearchResult:
"""
Search for help commands with filtering and pagination.
@ -316,23 +329,23 @@ class HelpCommandsService(BaseService[HelpCommand]):
# Apply filters
if filters.name_contains:
params.append(('name', filters.name_contains)) # API will do ILIKE search
params.append(("name", filters.name_contains)) # API will do ILIKE search
if filters.category:
params.append(('category', filters.category))
params.append(("category", filters.category))
params.append(('is_active', filters.is_active))
params.append(("is_active", filters.is_active))
# Add sorting
params.append(('sort', filters.sort_by))
params.append(("sort", filters.sort_by))
# Add pagination
params.append(('page', filters.page))
params.append(('page_size', filters.page_size))
params.append(("page", filters.page))
params.append(("page_size", filters.page_size))
# Execute search via API
client = await self.get_client()
data = await client.get('help_commands', params=params)
data = await client.get("help_commands", params=params)
if not data:
return HelpCommandSearchResult(
@ -341,14 +354,14 @@ class HelpCommandsService(BaseService[HelpCommand]):
page=filters.page,
page_size=filters.page_size,
total_pages=0,
has_more=False
has_more=False,
)
# Extract response data
help_commands_data = data.get('help_commands', [])
total_count = data.get('total_count', 0)
total_pages = data.get('total_pages', 0)
has_more = data.get('has_more', False)
help_commands_data = data.get("help_commands", [])
total_count = data.get("total_count", 0)
total_pages = data.get("total_pages", 0)
has_more = data.get("has_more", False)
# Convert to HelpCommand objects
help_commands = []
@ -356,15 +369,21 @@ class HelpCommandsService(BaseService[HelpCommand]):
try:
help_commands.append(self.model_class.from_api_data(cmd_data))
except Exception as e:
self.logger.warning("Failed to create HelpCommand from API data",
help_id=cmd_data.get('id'),
error=e)
self.logger.warning(
"Failed to create HelpCommand from API data",
help_id=cmd_data.get("id"),
error=e,
)
continue
self.logger.debug("Help commands search completed",
total_results=total_count,
page=filters.page,
filters_applied=len([p for p in params if p[0] not in ['sort', 'page', 'page_size']]))
self.logger.debug(
"Help commands search completed",
total_results=total_count,
page=filters.page,
filters_applied=len(
[p for p in params if p[0] not in ["sort", "page", "page_size"]]
),
)
return HelpCommandSearchResult(
help_commands=help_commands,
@ -372,13 +391,11 @@ class HelpCommandsService(BaseService[HelpCommand]):
page=filters.page,
page_size=filters.page_size,
total_pages=total_pages,
has_more=has_more
has_more=has_more,
)
async def get_all_help_topics(
self,
category: Optional[str] = None,
include_inactive: bool = False
self, category: Optional[str] = None, include_inactive: bool = False
) -> List[HelpCommand]:
"""
Get all help topics, optionally filtered by category.
@ -393,37 +410,36 @@ class HelpCommandsService(BaseService[HelpCommand]):
params = []
if category:
params.append(('category', category))
params.append(("category", category))
params.append(('is_active', not include_inactive))
params.append(('sort', 'display_order'))
params.append(('page_size', 100)) # Get all
params.append(("is_active", not include_inactive))
params.append(("sort", "display_order"))
params.append(("page_size", 100)) # Get all
client = await self.get_client()
data = await client.get('help_commands', params=params)
data = await client.get("help_commands", params=params)
if not data:
return []
help_commands_data = data.get('help_commands', [])
help_commands_data = data.get("help_commands", [])
help_commands = []
for cmd_data in help_commands_data:
try:
help_commands.append(self.model_class.from_api_data(cmd_data))
except Exception as e:
self.logger.warning("Failed to create HelpCommand from API data",
help_id=cmd_data.get('id'),
error=e)
self.logger.warning(
"Failed to create HelpCommand from API data",
help_id=cmd_data.get("id"),
error=e,
)
continue
return help_commands
async def get_help_names_for_autocomplete(
self,
partial_name: str = "",
limit: int = 25,
include_inactive: bool = False
self, partial_name: str = "", limit: int = 25, include_inactive: bool = False
) -> List[str]:
"""
Get help command names for Discord autocomplete.
@ -439,25 +455,28 @@ class HelpCommandsService(BaseService[HelpCommand]):
try:
# Use the dedicated autocomplete endpoint
client = await self.get_client()
params = [('limit', limit)]
params = [("limit", limit)]
if partial_name:
params.append(('q', partial_name.lower()))
params.append(("q", partial_name.lower()))
result = await client.get('help_commands/autocomplete', params=params)
result = await client.get("help_commands/autocomplete", params=params)
# The autocomplete endpoint returns results with name, title, category
if isinstance(result, dict) and 'results' in result:
return [item['name'] for item in result['results']]
if isinstance(result, dict) and "results" in result:
return [item["name"] for item in result["results"]]
else:
self.logger.warning("Unexpected autocomplete response format",
response=result)
self.logger.warning(
"Unexpected autocomplete response format", response=result
)
return []
except Exception as e:
self.logger.error("Failed to get help names for autocomplete",
partial_name=partial_name,
error=e)
self.logger.error(
"Failed to get help names for autocomplete",
partial_name=partial_name,
error=e,
)
# Return empty list on error to not break Discord autocomplete
return []
@ -467,7 +486,7 @@ class HelpCommandsService(BaseService[HelpCommand]):
"""Get comprehensive statistics about help commands."""
try:
client = await self.get_client()
data = await client.get('help_commands/stats')
data = await client.get("help_commands/stats")
if not data:
return HelpCommandStats(
@ -475,23 +494,25 @@ class HelpCommandsService(BaseService[HelpCommand]):
active_commands=0,
total_views=0,
most_viewed_command=None,
recent_commands_count=0
recent_commands_count=0,
)
# Convert most_viewed_command if present
most_viewed = None
if data.get('most_viewed_command'):
if data.get("most_viewed_command"):
try:
most_viewed = self.model_class.from_api_data(data['most_viewed_command'])
most_viewed = self.model_class.from_api_data(
data["most_viewed_command"]
)
except Exception as e:
self.logger.warning("Failed to parse most viewed command", error=e)
return HelpCommandStats(
total_commands=data.get('total_commands', 0),
active_commands=data.get('active_commands', 0),
total_views=data.get('total_views', 0),
total_commands=data.get("total_commands", 0),
active_commands=data.get("active_commands", 0),
total_views=data.get("total_views", 0),
most_viewed_command=most_viewed,
recent_commands_count=data.get('recent_commands_count', 0)
recent_commands_count=data.get("recent_commands_count", 0),
)
except Exception as e:
@ -502,7 +523,7 @@ class HelpCommandsService(BaseService[HelpCommand]):
active_commands=0,
total_views=0,
most_viewed_command=None,
recent_commands_count=0
recent_commands_count=0,
)

View File

@ -277,6 +277,35 @@ class TransactionBuilder:
Returns:
Tuple of (success: bool, error_message: str). If success is True, error_message is empty.
"""
# Fetch current state once if needed by FA lock or pending-transaction check
is_fa_pickup = (
move.from_roster == RosterType.FREE_AGENCY
and move.to_roster != RosterType.FREE_AGENCY
)
needs_current_state = is_fa_pickup or (
check_pending_transactions and next_week is None
)
current_week: Optional[int] = None
if needs_current_state:
try:
current_state = await league_service.get_current_state()
current_week = current_state.week if current_state else 1
except Exception as e:
logger.warning(f"Could not get current week: {e}")
current_week = 1
# Block adding players FROM Free Agency after the FA lock deadline
if is_fa_pickup and current_week is not None:
config = get_config()
if current_week >= config.fa_lock_week:
error_msg = (
f"Free agency is closed (week {current_week}, deadline was week {config.fa_lock_week}). "
f"Cannot add {move.player.name} from FA."
)
logger.warning(error_msg)
return False, error_msg
# Check if player is already in a move in this transaction builder
existing_move = self.get_move_for_player(move.player.id)
if existing_move:
@ -299,23 +328,15 @@ class TransactionBuilder:
return False, error_msg
# Check if player is already in another team's pending transaction for next week
# This prevents duplicate claims that would need to be resolved at freeze time
# Only applies to /dropadd (scheduled moves), not /ilmove (immediate moves)
if check_pending_transactions:
if next_week is None:
try:
current_state = await league_service.get_current_state()
next_week = (current_state.week + 1) if current_state else 1
except Exception as e:
logger.warning(
f"Could not get current week for pending transaction check: {e}"
)
next_week = 1
next_week = (current_week + 1) if current_week else 1
is_pending, claiming_team = (
await transaction_service.is_player_in_pending_transaction(
player_id=move.player.id, week=next_week, season=self.season
)
(
is_pending,
claiming_team,
) = await transaction_service.is_player_in_pending_transaction(
player_id=move.player.id, week=next_week, season=self.season
)
if is_pending:

View File

@ -3,6 +3,7 @@ Tests for Help Commands Service in Discord Bot v2.0
Comprehensive tests for help commands CRUD operations and business logic.
"""
import pytest
from datetime import datetime, timezone
from unittest.mock import AsyncMock
@ -10,13 +11,13 @@ from unittest.mock import AsyncMock
from services.help_commands_service import (
HelpCommandsService,
HelpCommandNotFoundError,
HelpCommandExistsError
HelpCommandExistsError,
)
from models.help_command import (
HelpCommand,
HelpCommandSearchFilters,
HelpCommandSearchResult,
HelpCommandStats
HelpCommandStats,
)
@ -26,17 +27,17 @@ def sample_help_command() -> HelpCommand:
now = datetime.now(timezone.utc)
return HelpCommand(
id=1,
name='trading-rules',
title='Trading Rules & Guidelines',
content='Complete trading rules for the league...',
category='rules',
created_by_discord_id='123456789',
name="trading-rules",
title="Trading Rules & Guidelines",
content="Complete trading rules for the league...",
category="rules",
created_by_discord_id="123456789",
created_at=now,
updated_at=None,
last_modified_by=None,
is_active=True,
view_count=100,
display_order=10
display_order=10,
)
@ -64,6 +65,7 @@ class TestHelpCommandsServiceInit:
# Multiple imports should return the same instance
from services.help_commands_service import help_commands_service as service2
assert help_commands_service is service2
def test_service_has_required_methods(self):
@ -71,22 +73,22 @@ class TestHelpCommandsServiceInit:
from services.help_commands_service import help_commands_service
# Core CRUD operations
assert hasattr(help_commands_service, 'create_help')
assert hasattr(help_commands_service, 'get_help_by_name')
assert hasattr(help_commands_service, 'update_help')
assert hasattr(help_commands_service, 'delete_help')
assert hasattr(help_commands_service, 'restore_help')
assert hasattr(help_commands_service, "create_help")
assert hasattr(help_commands_service, "get_help_by_name")
assert hasattr(help_commands_service, "update_help")
assert hasattr(help_commands_service, "delete_help")
assert hasattr(help_commands_service, "restore_help")
# Search and listing
assert hasattr(help_commands_service, 'search_help_commands')
assert hasattr(help_commands_service, 'get_all_help_topics')
assert hasattr(help_commands_service, 'get_help_names_for_autocomplete')
assert hasattr(help_commands_service, "search_help_commands")
assert hasattr(help_commands_service, "get_all_help_topics")
assert hasattr(help_commands_service, "get_help_names_for_autocomplete")
# View tracking
assert hasattr(help_commands_service, 'increment_view_count')
assert hasattr(help_commands_service, "increment_view_count")
# Statistics
assert hasattr(help_commands_service, 'get_statistics')
assert hasattr(help_commands_service, "get_statistics")
class TestHelpCommandsServiceCRUD:
@ -118,7 +120,7 @@ class TestHelpCommandsServiceCRUD:
last_modified_by=None,
is_active=True,
view_count=0,
display_order=data.get("display_order", 0)
display_order=data.get("display_order", 0),
)
return created_help
@ -130,8 +132,8 @@ class TestHelpCommandsServiceCRUD:
name="test-topic",
title="Test Topic",
content="This is test content for the help topic.",
creator_discord_id='123456789',
category="info"
creator_discord_id="123456789",
category="info",
)
assert isinstance(result, HelpCommand)
@ -141,39 +143,48 @@ class TestHelpCommandsServiceCRUD:
assert result.view_count == 0
@pytest.mark.asyncio
async def test_create_help_already_exists(self, help_commands_service_instance, sample_help_command):
async def test_create_help_already_exists(
self, help_commands_service_instance, sample_help_command
):
"""Test help command creation when topic already exists."""
# Mock topic already exists
async def mock_get_help_by_name(*args, **kwargs):
return sample_help_command
help_commands_service_instance.get_help_by_name = mock_get_help_by_name
with pytest.raises(HelpCommandExistsError, match="Help topic 'trading-rules' already exists"):
with pytest.raises(
HelpCommandExistsError, match="Help topic 'trading-rules' already exists"
):
await help_commands_service_instance.create_help(
name="trading-rules",
title="Trading Rules",
content="Rules content",
creator_discord_id='123456789'
creator_discord_id="123456789",
)
@pytest.mark.asyncio
async def test_get_help_by_name_success(self, help_commands_service_instance, sample_help_command):
async def test_get_help_by_name_success(
self, help_commands_service_instance, sample_help_command
):
"""Test successful help command retrieval."""
# Mock the API client to return proper data structure
help_data = {
'id': sample_help_command.id,
'name': sample_help_command.name,
'title': sample_help_command.title,
'content': sample_help_command.content,
'category': sample_help_command.category,
'created_by_discord_id': sample_help_command.created_by_discord_id,
'created_at': sample_help_command.created_at.isoformat(),
'updated_at': sample_help_command.updated_at.isoformat() if sample_help_command.updated_at else None,
'last_modified_by': sample_help_command.last_modified_by,
'is_active': sample_help_command.is_active,
'view_count': sample_help_command.view_count,
'display_order': sample_help_command.display_order
"id": sample_help_command.id,
"name": sample_help_command.name,
"title": sample_help_command.title,
"content": sample_help_command.content,
"category": sample_help_command.category,
"created_by_discord_id": sample_help_command.created_by_discord_id,
"created_at": sample_help_command.created_at.isoformat(),
"updated_at": sample_help_command.updated_at.isoformat()
if sample_help_command.updated_at
else None,
"last_modified_by": sample_help_command.last_modified_by,
"is_active": sample_help_command.is_active,
"view_count": sample_help_command.view_count,
"display_order": sample_help_command.display_order,
}
help_commands_service_instance._client.get.return_value = help_data
@ -191,66 +202,61 @@ class TestHelpCommandsServiceCRUD:
# Mock the API client to return None (not found)
help_commands_service_instance._client.get.return_value = None
with pytest.raises(HelpCommandNotFoundError, match="Help topic 'nonexistent' not found"):
with pytest.raises(
HelpCommandNotFoundError, match="Help topic 'nonexistent' not found"
):
await help_commands_service_instance.get_help_by_name("nonexistent")
@pytest.mark.asyncio
async def test_update_help_success(self, help_commands_service_instance, sample_help_command):
async def test_update_help_success(
self, help_commands_service_instance, sample_help_command
):
"""Test successful help command update."""
# Mock getting the existing help command
async def mock_get_help_by_name(name, include_inactive=False):
if name == "trading-rules":
return sample_help_command
raise HelpCommandNotFoundError(f"Help topic '{name}' not found")
# Mock the API update call
# Mock the API update call returning the updated help command data directly
updated_data = {
"id": sample_help_command.id,
"name": sample_help_command.name,
"title": "Updated Trading Rules",
"content": "Updated content",
"category": sample_help_command.category,
"created_by_discord_id": sample_help_command.created_by_discord_id,
"created_at": sample_help_command.created_at.isoformat(),
"updated_at": datetime.now(timezone.utc).isoformat(),
"last_modified_by": "987654321",
"is_active": sample_help_command.is_active,
"view_count": sample_help_command.view_count,
"display_order": sample_help_command.display_order,
}
async def mock_put(*args, **kwargs):
return True
return updated_data
help_commands_service_instance.get_help_by_name = mock_get_help_by_name
help_commands_service_instance._client.put = mock_put
# Update should call get_help_by_name again at the end, so mock it to return updated version
updated_help = HelpCommand(
id=sample_help_command.id,
name=sample_help_command.name,
title="Updated Trading Rules",
content="Updated content",
category=sample_help_command.category,
created_by_discord_id=sample_help_command.created_by_discord_id,
created_at=sample_help_command.created_at,
updated_at=datetime.now(timezone.utc),
last_modified_by='987654321',
is_active=sample_help_command.is_active,
view_count=sample_help_command.view_count,
display_order=sample_help_command.display_order
)
call_count = 0
async def mock_get_with_counter(name, include_inactive=False):
nonlocal call_count
call_count += 1
if call_count == 1:
return sample_help_command
else:
return updated_help
help_commands_service_instance.get_help_by_name = mock_get_with_counter
result = await help_commands_service_instance.update_help(
name="trading-rules",
new_title="Updated Trading Rules",
new_content="Updated content",
updater_discord_id='987654321'
updater_discord_id="987654321",
)
assert isinstance(result, HelpCommand)
assert result.title == "Updated Trading Rules"
@pytest.mark.asyncio
async def test_delete_help_success(self, help_commands_service_instance, sample_help_command):
async def test_delete_help_success(
self, help_commands_service_instance, sample_help_command
):
"""Test successful help command deletion (soft delete)."""
# Mock getting the help command
async def mock_get_help_by_name(name, include_inactive=False):
return sample_help_command
@ -272,12 +278,12 @@ class TestHelpCommandsServiceCRUD:
# Mock getting a deleted help command
deleted_help = HelpCommand(
id=1,
name='deleted-topic',
title='Deleted Topic',
content='Content',
created_by_discord_id='123456789',
name="deleted-topic",
title="Deleted Topic",
content="Content",
created_by_discord_id="123456789",
created_at=datetime.now(timezone.utc),
is_active=False
is_active=False,
)
async def mock_get_help_by_name(name, include_inactive=False):
@ -285,15 +291,15 @@ class TestHelpCommandsServiceCRUD:
# Mock the API restore call
restored_data = {
'id': deleted_help.id,
'name': deleted_help.name,
'title': deleted_help.title,
'content': deleted_help.content,
'created_by_discord_id': deleted_help.created_by_discord_id,
'created_at': deleted_help.created_at.isoformat(),
'is_active': True,
'view_count': 0,
'display_order': 0
"id": deleted_help.id,
"name": deleted_help.name,
"title": deleted_help.title,
"content": deleted_help.content,
"created_by_discord_id": deleted_help.created_by_discord_id,
"created_at": deleted_help.created_at.isoformat(),
"is_active": True,
"view_count": 0,
"display_order": 0,
}
help_commands_service_instance.get_help_by_name = mock_get_help_by_name
@ -312,33 +318,30 @@ class TestHelpCommandsServiceSearch:
async def test_search_help_commands(self, help_commands_service_instance):
"""Test searching for help commands with filters."""
filters = HelpCommandSearchFilters(
name_contains='trading',
category='rules',
page=1,
page_size=10
name_contains="trading", category="rules", page=1, page_size=10
)
# Mock API response
api_response = {
'help_commands': [
"help_commands": [
{
'id': 1,
'name': 'trading-rules',
'title': 'Trading Rules',
'content': 'Content',
'category': 'rules',
'created_by_discord_id': '123',
'created_at': datetime.now(timezone.utc).isoformat(),
'is_active': True,
'view_count': 100,
'display_order': 0
"id": 1,
"name": "trading-rules",
"title": "Trading Rules",
"content": "Content",
"category": "rules",
"created_by_discord_id": "123",
"created_at": datetime.now(timezone.utc).isoformat(),
"is_active": True,
"view_count": 100,
"display_order": 0,
}
],
'total_count': 1,
'page': 1,
'page_size': 10,
'total_pages': 1,
'has_more': False
"total_count": 1,
"page": 1,
"page_size": 10,
"total_pages": 1,
"has_more": False,
}
help_commands_service_instance._client.get.return_value = api_response
@ -348,33 +351,33 @@ class TestHelpCommandsServiceSearch:
assert isinstance(result, HelpCommandSearchResult)
assert len(result.help_commands) == 1
assert result.total_count == 1
assert result.help_commands[0].name == 'trading-rules'
assert result.help_commands[0].name == "trading-rules"
@pytest.mark.asyncio
async def test_get_all_help_topics(self, help_commands_service_instance):
"""Test getting all help topics."""
# Mock API response
api_response = {
'help_commands': [
"help_commands": [
{
'id': i,
'name': f'topic-{i}',
'title': f'Topic {i}',
'content': f'Content {i}',
'category': 'rules' if i % 2 == 0 else 'guides',
'created_by_discord_id': '123',
'created_at': datetime.now(timezone.utc).isoformat(),
'is_active': True,
'view_count': i * 10,
'display_order': i
"id": i,
"name": f"topic-{i}",
"title": f"Topic {i}",
"content": f"Content {i}",
"category": "rules" if i % 2 == 0 else "guides",
"created_by_discord_id": "123",
"created_at": datetime.now(timezone.utc).isoformat(),
"is_active": True,
"view_count": i * 10,
"display_order": i,
}
for i in range(1, 6)
],
'total_count': 5,
'page': 1,
'page_size': 100,
'total_pages': 1,
'has_more': False
"total_count": 5,
"page": 1,
"page_size": 100,
"total_pages": 1,
"has_more": False,
}
help_commands_service_instance._client.get.return_value = api_response
@ -386,42 +389,45 @@ class TestHelpCommandsServiceSearch:
assert all(isinstance(cmd, HelpCommand) for cmd in result)
@pytest.mark.asyncio
async def test_get_help_names_for_autocomplete(self, help_commands_service_instance):
async def test_get_help_names_for_autocomplete(
self, help_commands_service_instance
):
"""Test getting help names for autocomplete."""
# Mock API response
api_response = {
'results': [
"results": [
{
'name': 'trading-rules',
'title': 'Trading Rules',
'category': 'rules'
"name": "trading-rules",
"title": "Trading Rules",
"category": "rules",
},
{
'name': 'trading-deadline',
'title': 'Trading Deadline',
'category': 'info'
}
"name": "trading-deadline",
"title": "Trading Deadline",
"category": "info",
},
]
}
help_commands_service_instance._client.get.return_value = api_response
result = await help_commands_service_instance.get_help_names_for_autocomplete(
partial_name='trading',
limit=25
partial_name="trading", limit=25
)
assert isinstance(result, list)
assert len(result) == 2
assert 'trading-rules' in result
assert 'trading-deadline' in result
assert "trading-rules" in result
assert "trading-deadline" in result
class TestHelpCommandsServiceViewTracking:
"""Test view count tracking."""
@pytest.mark.asyncio
async def test_increment_view_count(self, help_commands_service_instance, sample_help_command):
async def test_increment_view_count(
self, help_commands_service_instance, sample_help_command
):
"""Test incrementing view count."""
# Mock the API patch call
help_commands_service_instance._client.patch = AsyncMock()
@ -437,7 +443,7 @@ class TestHelpCommandsServiceViewTracking:
created_at=sample_help_command.created_at,
is_active=sample_help_command.is_active,
view_count=sample_help_command.view_count + 1,
display_order=sample_help_command.display_order
display_order=sample_help_command.display_order,
)
async def mock_get_help_by_name(name, include_inactive=False):
@ -445,7 +451,9 @@ class TestHelpCommandsServiceViewTracking:
help_commands_service_instance.get_help_by_name = mock_get_help_by_name
result = await help_commands_service_instance.increment_view_count("trading-rules")
result = await help_commands_service_instance.increment_view_count(
"trading-rules"
)
assert isinstance(result, HelpCommand)
assert result.view_count == 101
@ -459,21 +467,21 @@ class TestHelpCommandsServiceStatistics:
"""Test getting help command statistics."""
# Mock API response
api_response = {
'total_commands': 50,
'active_commands': 45,
'total_views': 5000,
'most_viewed_command': {
'id': 1,
'name': 'popular-topic',
'title': 'Popular Topic',
'content': 'Content',
'created_by_discord_id': '123',
'created_at': datetime.now(timezone.utc).isoformat(),
'is_active': True,
'view_count': 500,
'display_order': 0
"total_commands": 50,
"active_commands": 45,
"total_views": 5000,
"most_viewed_command": {
"id": 1,
"name": "popular-topic",
"title": "Popular Topic",
"content": "Content",
"created_by_discord_id": "123",
"created_at": datetime.now(timezone.utc).isoformat(),
"is_active": True,
"view_count": 500,
"display_order": 0,
},
'recent_commands_count': 5
"recent_commands_count": 5,
}
help_commands_service_instance._client.get.return_value = api_response
@ -485,7 +493,7 @@ class TestHelpCommandsServiceStatistics:
assert result.active_commands == 45
assert result.total_views == 5000
assert result.most_viewed_command is not None
assert result.most_viewed_command.name == 'popular-topic'
assert result.most_viewed_command.name == "popular-topic"
assert result.recent_commands_count == 5
@ -498,7 +506,9 @@ class TestHelpCommandsServiceErrorHandling:
from exceptions import APIException, BotException
# Mock the API client to raise an APIException
help_commands_service_instance._client.get.side_effect = APIException("Connection error")
help_commands_service_instance._client.get.side_effect = APIException(
"Connection error"
)
with pytest.raises(BotException, match="Failed to retrieve help topic 'test'"):
await help_commands_service_instance.get_help_by_name("test")

View File

@ -115,6 +115,13 @@ class TestTransactionBuilder:
svc.get_current_roster.return_value = mock_roster
return svc
@pytest.fixture(autouse=True)
def mock_league_service(self):
"""Patch league_service for all tests so FA lock check uses week 10 (before deadline)."""
with patch("services.transaction_builder.league_service") as mock_ls:
mock_ls.get_current_state = AsyncMock(return_value=MagicMock(week=10))
yield mock_ls
@pytest.fixture
def builder(self, mock_team, mock_roster_service):
"""Create a TransactionBuilder for testing with injected roster service."""
@ -152,6 +159,50 @@ class TestTransactionBuilder:
assert builder.is_empty is False
assert move in builder.moves
@pytest.mark.asyncio
async def test_add_move_from_fa_blocked_after_deadline(self, builder, mock_player):
"""Test that adding a player FROM Free Agency is blocked after fa_lock_week."""
move = TransactionMove(
player=mock_player,
from_roster=RosterType.FREE_AGENCY,
to_roster=RosterType.MAJOR_LEAGUE,
to_team=builder.team,
)
with patch(
"services.transaction_builder.league_service"
) as mock_league_service:
mock_league_service.get_current_state = AsyncMock(
return_value=MagicMock(week=15)
)
success, error_message = await builder.add_move(
move, check_pending_transactions=False
)
assert success is False
assert "Free agency is closed" in error_message
assert builder.move_count == 0
@pytest.mark.asyncio
async def test_drop_to_fa_allowed_after_deadline(self, builder, mock_player):
"""Test that dropping a player TO Free Agency is still allowed after fa_lock_week."""
move = TransactionMove(
player=mock_player,
from_roster=RosterType.MAJOR_LEAGUE,
to_roster=RosterType.FREE_AGENCY,
from_team=builder.team,
)
# Drop to FA doesn't trigger the FA lock check (autouse fixture provides week 10)
success, error_message = await builder.add_move(
move, check_pending_transactions=False
)
assert success is True
assert error_message == ""
assert builder.move_count == 1
@pytest.mark.asyncio
async def test_add_duplicate_move_fails(self, builder, mock_player):
"""Test that adding duplicate moves for same player fails."""
@ -809,6 +860,13 @@ class TestPendingTransactionValidation:
"""Create a mock player for testing."""
return Player(id=12472, name="Test Player", wara=2.5, season=12, pos_1="OF")
@pytest.fixture(autouse=True)
def mock_league_service(self):
"""Patch league_service so FA lock check and week resolution use week 10."""
with patch("services.transaction_builder.league_service") as mock_ls:
mock_ls.get_current_state = AsyncMock(return_value=MagicMock(week=10))
yield mock_ls
@pytest.fixture
def builder(self, mock_team):
"""Create a TransactionBuilder for testing."""

View File

@ -344,6 +344,7 @@ class TradeAcceptanceView(discord.ui.View):
def __init__(self, builder: TradeBuilder):
super().__init__(timeout=3600.0) # 1 hour timeout
self.builder = builder
self._checked_teams: dict[int, Team] = {}
async def _get_user_team(self, interaction: discord.Interaction) -> Optional[Team]:
"""Get the team owned by the interacting user."""
@ -369,6 +370,7 @@ class TradeAcceptanceView(discord.ui.View):
)
return False
self._checked_teams[interaction.user.id] = user_team
return True
async def on_timeout(self) -> None:
@ -382,7 +384,7 @@ class TradeAcceptanceView(discord.ui.View):
self, interaction: discord.Interaction, button: discord.ui.Button
):
"""Handle accept button click."""
user_team = await self._get_user_team(interaction)
user_team = self._checked_teams.get(interaction.user.id)
if not user_team:
return
@ -417,7 +419,7 @@ class TradeAcceptanceView(discord.ui.View):
self, interaction: discord.Interaction, button: discord.ui.Button
):
"""Handle reject button click - moves trade back to DRAFT."""
user_team = await self._get_user_team(interaction)
user_team = self._checked_teams.get(interaction.user.id)
if not user_team:
return
@ -733,10 +735,10 @@ async def create_trade_embed(builder: TradeBuilder) -> discord.Embed:
Returns:
Discord embed with current trade state
"""
validation = await builder.validate_trade()
if builder.is_empty:
color = EmbedColors.SECONDARY
else:
validation = await builder.validate_trade()
color = EmbedColors.SUCCESS if validation.is_legal else EmbedColors.WARNING
embed = EmbedTemplate.create_base_embed(
@ -791,7 +793,6 @@ async def create_trade_embed(builder: TradeBuilder) -> discord.Embed:
inline=False,
)
validation = await builder.validate_trade()
if validation.is_legal:
status_text = "Trade appears legal"
else: