feat: adjust settings to use guild configs #86
Conversation
chore(ui): increase byte screen logo size
* feat: add guild configuration command * feat: add litestar office hours command * fix: centralize options for slashcmd and dropdown * ci: apply pre-commit * chore: dependecy updates * chore: clean up utils * fix: move moved import * fix: attempt to fix broken modal submission * feat: finalize working config selection and sub selections * ci: for some reason pre-commit is stupid and changed every fucking file
Reviewer's Guide by SourceryThis pull request implements significant changes to support per-guild settings in the Byte Discord bot. It includes modifications to database models, API endpoints, and bot commands to allow for more granular configuration of bot features on a per-guild basis. The changes also include improvements to the project structure, type hinting, and error handling. ER diagram for Guild ConfigurationerDiagram
GUILD {
UUID id
string guild_name
string prefix
int help_channel_id
int showcase_channel_id
string sync_label
boolean issue_linking
boolean comment_linking
boolean pep_linking
}
GITHUB_CONFIG {
int guild_id
boolean discussion_sync
string github_organization
string github_repository
}
SOTAGS_CONFIG {
int guild_id
string tag_name
}
ALLOWED_USERS_CONFIG {
int guild_id
UUID user_id
}
FORUM_CONFIG {
int guild_id
boolean help_forum
string help_forum_category
boolean help_thread_auto_close
int help_thread_auto_close_days
boolean help_thread_notify
string help_thread_notify_roles
int help_thread_notify_days
boolean showcase_forum
string showcase_forum_category
boolean showcase_thread_auto_close
int showcase_thread_auto_close_days
}
GUILD ||--o{ GITHUB_CONFIG : has
GUILD ||--o{ SOTAGS_CONFIG : has
GUILD ||--o{ ALLOWED_USERS_CONFIG : has
GUILD ||--o{ FORUM_CONFIG : has
Class diagram for Guild ConfigurationclassDiagram
class GuildsController {
+get_guild(guild_id: int) GuildSchema
+update_guild(guild_id: int, setting: UpdateableGuildSetting, value: str | int) GuildSchema
+get_guild_github_config(guild_id: int) GitHubConfigSchema
+get_guild_sotags_config(guild_id: int) SOTagsConfigSchema
+get_guild_allowed_users_config(guild_id: int) AllowedUsersConfigSchema
+get_guild_forum_config(guild_id: int) ForumConfigSchema
}
class GuildSchema {
+UUID guild_id
+bool issue_linking
+bool comment_linking
+bool pep_linking
+GitHubConfigSchema github_config
+list~SOTagsConfigSchema~ sotags_configs
+list~AllowedUsersConfigSchema~ allowed_users
+ForumConfigSchema forum_config
}
class GitHubConfigSchema {
+UUID guild_id
+bool discussion_sync
+str github_organization
+str github_repository
}
class SOTagsConfigSchema {
+UUID guild_id
+str tag_name
}
class AllowedUsersConfigSchema {
+UUID guild_id
+UUID user_id
}
class ForumConfigSchema {
+UUID guild_id
+bool help_forum
+str help_forum_category
+bool help_thread_auto_close
+int help_thread_auto_close_days
+bool help_thread_notify
+list~int~ help_thread_notify_roles
+int help_thread_notify_days
+bool help_thread_sync
+bool showcase_forum
+str showcase_forum_category
+bool showcase_thread_auto_close
+int showcase_thread_auto_close_days
}
GuildsController --> GuildSchema
GuildSchema --> GitHubConfigSchema
GuildSchema --> SOTagsConfigSchema
GuildSchema --> AllowedUsersConfigSchema
GuildSchema --> ForumConfigSchema
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @JacobCoffee - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟡 Documentation: 2 issues found
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -48,3 +115,75 @@ class GuildUpdate(CamelizedBaseModel): | |||
| issue_linking: bool | None = Field(title="Issue Linking", description="Is issue linking enabled.") | |||
| comment_linking: bool | None = Field(title="Comment Linking", description="Is comment linking enabled.") | |||
| pep_linking: bool | None = Field(title="PEP Linking", description="Is PEP linking enabled.") | |||
|
|
|||
|
|
|||
| class UpdateableGuildSetting(CamelizedBaseModel): | |||
There was a problem hiding this comment.
suggestion: Review and enhance validation for UpdateableGuildSetting
Ensure that the UpdateableGuildSetting class covers all possible settings and includes comprehensive validation logic. Consider adding more specific validators for each field to prevent invalid data.
| logger.info("successfully added guild %s (ID: %s)", guild.name, guild.id) | ||
| else: | ||
| logger.error("%s joined guild '%s' but was not added to database", self.user.name, guild.name) | ||
| try: |
There was a problem hiding this comment.
suggestion: Enhance error logging in on_guild_join method
While error handling has been added, consider including more detailed error messages in the logging statements. This will help with debugging and identifying the root cause of any issues that may occur during guild joining.
try:
async with httpx.AsyncClient() as client:
response = await client.post(api_url)
response.raise_for_status()
except httpx.HTTPError as e:
logger.error(f"HTTP error occurred while joining guild {guild.name} (ID: {guild.id}): {str(e)}")
except Exception as e:
logger.error(f"Unexpected error occurred while joining guild {guild.name} (ID: {guild.id}): {str(e)}")
| __all__ = ("get_byte_server_count",) | ||
|
|
||
|
|
||
| async def get_byte_server_count() -> int: |
There was a problem hiding this comment.
suggestion: Add error handling for database operations
Consider adding try-except blocks to handle potential database errors gracefully. This will make the function more robust and prevent unexpected crashes.
async def get_byte_server_count() -> int:
try:
# Existing code to get server count
pass
except Exception as e:
logger.error(f"Error fetching Byte server count: {e}")
return 0
| ===== | ||
| Usage |
There was a problem hiding this comment.
suggestion (documentation): Remove extra underlining for consistency in header formatting.
The 'Usage' header now has underlining both above and below. This isn't consistent with other headers in the documentation. Consider removing the top line of '=' signs to maintain consistency.
| ===== | |
| Usage | |
| Usage | |
| ===== |
| guild | ||
| testing |
There was a problem hiding this comment.
question (documentation): Clarify the removal of the 'guild' plugin documentation.
The 'guild' plugin documentation has been removed and replaced with 'python'. Can you confirm if the guild functionality has been removed or moved elsewhere? This change might impact users who were relying on the guild plugin documentation.
|
|
||
|
|
||
| class GuildController(Controller): | ||
| class GuildsController(Controller): |
There was a problem hiding this comment.
issue (complexity): Consider implementing a generic method and service for handling all configuration types.
The controller's complexity has increased significantly with the addition of multiple methods for different guild configurations. To reduce complexity while maintaining functionality, consider the following changes:
- Implement a generic method for handling all configuration types:
@get(
operation_id="ConfigDetail",
name="guilds:config",
summary="Get config for a guild.",
path="/guilds/{guild_id}/{config_type}",
)
async def get_guild_config(
self,
config_service: Annotated[ConfigService, Depends(get_config_service)],
guild_id: int = Parameter(title="Guild ID", description="The guild ID."),
config_type: str = Parameter(title="Config Type", description="The type of configuration to retrieve."),
) -> ConfigSchema | OffsetPagination[ConfigSchema]:
"""Get a guild's config by ID and type."""
result = await config_service.get(guild_id, config_type)
return config_service.to_schema(result)- Create a
ConfigServicethat handles all configuration types:
class ConfigService:
def __init__(self, config_services: dict[str, BaseConfigService]):
self.config_services = config_services
async def get(self, guild_id: int, config_type: str) -> Any:
service = self.config_services.get(config_type)
if not service:
raise ValueError(f"Invalid config type: {config_type}")
return await service.get(guild_id, id_attribute="guild_id")
def to_schema(self, result: Any) -> ConfigSchema | OffsetPagination[ConfigSchema]:
# Implementation depends on your specific needs
pass- Update the dependency injection to use the new
ConfigService:
def get_config_service():
return ConfigService({
"github": provides_github_config_service(),
"sotags": provides_sotags_config_service(),
"allowed_users": provides_allowed_users_config_service(),
"forum": provides_forum_config_service(),
})These changes will significantly reduce code duplication and complexity in the controller while maintaining the separation of concerns for different configuration types. The ConfigService acts as a facade, delegating to the appropriate service based on the config type. This approach is more scalable and easier to maintain as new configuration types are added in the future.
| batch_op.add_column(sa.Column("showcase_thread_auto_close", sa.Boolean(), nullable=False)) | ||
| batch_op.add_column(sa.Column("showcase_thread_auto_close_days", sa.Integer(), nullable=True)) | ||
| batch_op.add_column(sa.Column("sa_orm_sentinel", sa.Integer(), nullable=True)) | ||
| batch_op.add_column(sa.Column("created_at", sa.DateTimeUTC(timezone=True), nullable=False)) |
There was a problem hiding this comment.
issue (code-quality): Extract duplicate code into function (extract-duplicate-method)
Description
Summary by Sourcery
Implement per-guild configuration settings, allowing for customization of various guild-specific options. Add new API endpoints for managing these configurations and introduce a system health check endpoint. Update the bot's guild join process and enhance logging. Update CI to use Python 3.12 and add new Makefile targets for database management.
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests: