feat: adjust settings to use guild configs #87
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 several changes to adjust settings to use guild configurations. The changes include modifications to database models, API endpoints, and Discord bot functionality to support per-guild settings. The PR also introduces new configuration options and improves the overall structure of the project. Sequence diagram for updating guild settingssequenceDiagram
actor Admin
participant GuildsController
participant GuildsService
Admin->>GuildsController: PATCH /guilds/{guild_id}
GuildsController->>GuildsService: get(guild_id)
GuildsService-->>GuildsController: Guild
GuildsController->>GuildsService: update(guild, setting, value)
GuildsService-->>GuildsController: Updated Guild
GuildsController-->>Admin: Updated Guild Schema
Architecture diagram for Guild Configuration Servicesgraph TD;
subgraph GuildsService
A[GuildsService]
B[GitHubConfigService]
C[SOTagsConfigService]
D[AllowedUsersConfigService]
E[ForumConfigService]
end
A -->|Provides| B
A -->|Provides| C
A -->|Provides| D
A -->|Provides| E
subgraph Database
F[Guild]
G[GitHubConfig]
H[SOTagsConfig]
I[AllowedUsersConfig]
J[ForumConfig]
end
B -->|Interacts with| G
C -->|Interacts with| H
D -->|Interacts with| I
E -->|Interacts with| J
A -->|Interacts with| F
Class diagram for Guild ConfigurationclassDiagram
class Guild {
+UUID guild_id
+String guild_name
+String prefix
+int help_channel_id
+int showcase_channel_id
+String sync_label
+bool issue_linking
+bool comment_linking
+bool pep_linking
+GitHubConfig github_config
+SOTagsConfig[] sotags_configs
+AllowedUsersConfig[] allowed_users
+ForumConfig forum_config
}
class GitHubConfig {
+int guild_id
+bool discussion_sync
+String github_organization
+String github_repository
}
class SOTagsConfig {
+int guild_id
+String tag_name
}
class AllowedUsersConfig {
+int guild_id
+UUID user_id
}
class ForumConfig {
+int guild_id
+bool help_forum
+String help_forum_category
+int help_channel_id
+bool help_thread_auto_close
+int help_thread_auto_close_days
+bool help_thread_notify
+String help_thread_notify_roles
+int help_thread_notify_days
+bool help_thread_sync
+bool showcase_forum
+String showcase_forum_category
+int showcase_channel_id
+bool showcase_thread_auto_close
+int showcase_thread_auto_close_days
}
Guild --> GitHubConfig
Guild --> SOTagsConfig
Guild --> AllowedUsersConfig
Guild --> ForumConfig
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
🚅 Deployed to the byte-pr-87 environment in byte
|
![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 found some issues that need to be addressed.
Blocking issues:
- Hardcoded internal ID found in settings. (link)
Overall Comments:
- The changes look good overall, but consider breaking future large changes into smaller, more focused PRs for easier review and integration.
- Some of the more complex parts of the new code, particularly in the UI views, could benefit from additional comments explaining the logic flow.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🔴 Security: 1 blocking issue
- 🟢 Testing: all looks good
- 🟡 Complexity: 2 issues found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -49,3 +113,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.
issue (complexity): Consider refactoring the UpdateableGuildSetting class into smaller, more focused classes.
The changes introduce more structured configuration options, which is good for maintainability. However, the UpdateableGuildSetting class has become overly complex with too many responsibilities. Consider splitting it into smaller, more focused classes:
class BaseGuildSetting(CamelizedBaseModel):
"""Base class for guild settings."""
guild_id: UUID
class GuildModelSetting(BaseGuildSetting):
"""Settings for the guild model."""
prefix: str
help_channel_id: int
showcase_channel_id: int
sync_label: str
issue_linking: bool
comment_linking: bool
pep_linking: bool
class GitHubConfigSetting(BaseGuildSetting):
"""Settings for GitHub configuration."""
discussion_sync: bool
github_organization: str
github_repository: str
# Similar classes for StackOverflow, AllowedUsers, and Forum configs
class UpdateableGuildSetting(CamelizedBaseModel):
"""Allowed settings that admins can update for their guild."""
guild_model: GuildModelSetting
github_config: GitHubConfigSetting
# Other config classes...This approach maintains the structured configs while reducing complexity. It also eliminates the need for the dynamic enum creation method, which adds unnecessary abstraction. If you need an enum of all possible settings, consider creating it explicitly:
class GuildSettingField(StrEnum):
PREFIX = "prefix"
HELP_CHANNEL_ID = "help_channel_id"
# ... other fields ...This change will make the code more maintainable and easier to understand while preserving the new functionality.
| @@ -42,6 +42,8 @@ class DiscordSettings(BaseSettings): | |||
| """Discord Guild ID for development.""" | |||
| DEV_USER_ID: int | |||
| """Discord User ID for development.""" | |||
| DEV_GUILD_INTERNAL_ID: int = 1136100160510902272 | |||
There was a problem hiding this comment.
🚨 issue (security): Hardcoded internal ID found in settings.
Consider using environment variables or a configuration file to manage sensitive IDs.
| 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)
JacobCoffee
commented
Oct 11, 2024
JacobCoffee
commented
Oct 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
#74 but rebased against big refactor
Summary by Sourcery
Implement per-guild configuration settings, including GitHub and StackOverflow tags, and introduce a new command for scheduling Office Hours events. Add a health check endpoint to monitor system status and enhance the bot's configuration capabilities with a new command for guild admins. Improve logging and refactor guilds controller for better functionality.
New Features:
Enhancements:
Build:
Documentation:
Tests: