perf: eliminate redundant GET after create/update and parallelize stats (#95) #112
@ -120,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:
|
||||
"""
|
||||
@ -218,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
|
||||
@ -544,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
|
||||
@ -565,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.
|
||||
@ -618,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,
|
||||
|
||||
@ -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,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@ -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")
|
||||
|
||||
Loading…
Reference in New Issue
Block a user