Added options to customize logo, addon name and discord#476
Added options to customize logo, addon name and discord#476DryKillLogic wants to merge 1 commit intog0ldyy:developmentfrom
Conversation
WalkthroughAdds three UI customization settings (CUSTOM_LOGO_URL, CUSTOM_ADDON_NAME, CUSTOM_DISCORD_URL) across configuration defaults, AppSettings, startup logging, the /configure endpoint context normalization, and the main HTML template to enable conditional rendering of logo, addon name, and Discord link. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Client (Browser)
participant Server as Comet API
participant Settings as AppSettings
participant Template as Jinja Template
Browser->>Server: GET /configure (page load)
Server->>Settings: read CUSTOM_* values
Settings-->>Server: return values (may be None/"none")
Server->>Server: normalize CUSTOM_* (convert None/"none" -> "")
Server->>Template: render index.html with html_context (**html_context) + webConfig
Template-->>Server: rendered HTML (logo/name/discord rendered conditionally)
Server-->>Browser: HTTP 200 + rendered page
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
🔇 Additional comments (3)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@comet/api/endpoints/config.py`:
- Around line 31-36: The template is rendering <img src=""> when normalize()
turns CUSTOM_LOGO_URL into an empty string; either update the template to
conditionally render the logo (wrap the <img ...> tag in a {% if CUSTOM_LOGO_URL
%} ... {% endif %} similar to CUSTOM_DISCORD_URL) or change normalize(v) to
return None instead of "" for None/"none"/"null" so the falsy check will omit
the key; update references to CUSTOM_LOGO_URL and the normalize function
accordingly.
In `@comet/core/logger.py`:
- Around line 446-448: The current logger.log calls (logger.log in
comet/core/logger.py) use bool(settings.CUSTOM_LOGO_URL),
bool(settings.CUSTOM_ADDON_NAME), and bool(settings.CUSTOM_DISCORD_URL) which
always evaluate True because those settings have non-None defaults; change the
logging to either log the actual values (truncated if long) or compare against
the real defaults (e.g., DEFAULT_LOGO_URL / DEFAULT_ADDON_NAME constants) so the
log reflects whether the value differs from the default—update the three
logger.log calls to use settings.CUSTOM_... != DEFAULT_... or to log the actual
setting string and keep the existing CUSTOM_HEADER_HTML behavior for reference.
In `@comet/templates/index.html`:
- Around line 385-390: The template currently renders the img tag using
CUSTOM_LOGO_URL unconditionally which produces a broken image when
CUSTOM_LOGO_URL is empty; update the template to conditionally render the <img>
element only when CUSTOM_LOGO_URL has a value (same pattern used for
CUSTOM_DISCORD_URL) so that when CUSTOM_LOGO_URL is None/empty no <img src="">
is emitted and the layout falls back to just showing CUSTOM_ADDON_NAME.
🧹 Nitpick comments (1)
.env-sample (1)
264-266: Clarify that=Nonedisables the default, not inherits it.Setting
CUSTOM_LOGO_URL=Nonewill pass the literal string"None"which gets normalized to""(empty string) inconfig.py. This would remove the logo rather than use the default.Consider using commented examples or clearer placeholder values:
Suggested improvement
-CUSTOM_LOGO_URL=None -CUSTOM_ADDON_NAME=None -CUSTOM_DISCORD_URL=None +# CUSTOM_LOGO_URL=https://your-logo-url.com/logo.png # Default: Comet emoji +# CUSTOM_ADDON_NAME=My Custom Addon # Default: Comet +# CUSTOM_DISCORD_URL=https://discord.com/invite/your-server # Default: Comet Discord (leave empty to hide button)This aligns with the file header guidance: "Only copy and modify the variables you need to change."
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.env-samplecomet/api/endpoints/config.pycomet/core/logger.pycomet/core/models.pycomet/templates/index.html
🔇 Additional comments (3)
comet/api/endpoints/config.py (1)
24-38: LGTM - Clean normalization approach.The
normalizefunction correctly handles the edge cases where environment variables are set to"None"or"null"strings. The approach of buildinghtml_contextand spreading it into the template context is clean.Minor optimization: the
normalizefunction could be moved outsideconfigure()to avoid recreation on each request, but this is negligible for a configuration page.comet/core/models.py (1)
110-112: LGTM - Sensible defaults for backwards compatibility.The defaults preserve the original Comet branding when these settings are not configured, which is the right approach for existing deployments. Users who want to customize can override, and those who want to hide the Discord button can set
CUSTOM_DISCORD_URLtoNoneor empty.comet/templates/index.html (1)
392-408: LGTM - Conditional Discord button rendering.The
{% if CUSTOM_DISCORD_URL %}wrapper correctly hides the Discord button when the URL is empty or not configured. This allows operators to remove the Discord link from their deployments.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| logger.log("COMET", f"Custom Logo URL: {bool(settings.CUSTOM_LOGO_URL)}") | ||
| logger.log("COMET", f"Custom Addon Name: {bool(settings.CUSTOM_ADDON_NAME)}") | ||
| logger.log("COMET", f"Custom Discord URL: {bool(settings.CUSTOM_DISCORD_URL)}") |
There was a problem hiding this comment.
Logging always shows True due to non-None defaults.
These settings have hardcoded defaults in models.py (e.g., CUSTOM_ADDON_NAME = "Comet"), so bool(settings.CUSTOM_...) will always evaluate to True. This differs from CUSTOM_HEADER_HTML on line 449, which defaults to None and thus correctly shows False when unconfigured.
Consider checking whether the value differs from the default, or simply log the actual value (truncated if needed) for better operator visibility:
Suggested approach
- logger.log("COMET", f"Custom Logo URL: {bool(settings.CUSTOM_LOGO_URL)}")
- logger.log("COMET", f"Custom Addon Name: {bool(settings.CUSTOM_ADDON_NAME)}")
- logger.log("COMET", f"Custom Discord URL: {bool(settings.CUSTOM_DISCORD_URL)}")
+ logger.log("COMET", f"Custom Logo URL: {settings.CUSTOM_LOGO_URL}")
+ logger.log("COMET", f"Custom Addon Name: {settings.CUSTOM_ADDON_NAME}")
+ logger.log("COMET", f"Custom Discord URL: {settings.CUSTOM_DISCORD_URL}")Alternatively, define the defaults as constants and check settings.CUSTOM_LOGO_URL != DEFAULT_LOGO_URL etc.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.log("COMET", f"Custom Logo URL: {bool(settings.CUSTOM_LOGO_URL)}") | |
| logger.log("COMET", f"Custom Addon Name: {bool(settings.CUSTOM_ADDON_NAME)}") | |
| logger.log("COMET", f"Custom Discord URL: {bool(settings.CUSTOM_DISCORD_URL)}") | |
| logger.log("COMET", f"Custom Logo URL: {settings.CUSTOM_LOGO_URL}") | |
| logger.log("COMET", f"Custom Addon Name: {settings.CUSTOM_ADDON_NAME}") | |
| logger.log("COMET", f"Custom Discord URL: {settings.CUSTOM_DISCORD_URL}") |
🤖 Prompt for AI Agents
In `@comet/core/logger.py` around lines 446 - 448, The current logger.log calls
(logger.log in comet/core/logger.py) use bool(settings.CUSTOM_LOGO_URL),
bool(settings.CUSTOM_ADDON_NAME), and bool(settings.CUSTOM_DISCORD_URL) which
always evaluate True because those settings have non-None defaults; change the
logging to either log the actual values (truncated if long) or compare against
the real defaults (e.g., DEFAULT_LOGO_URL / DEFAULT_ADDON_NAME constants) so the
log reflects whether the value differs from the default—update the three
logger.log calls to use settings.CUSTOM_... != DEFAULT_... or to log the actual
setting string and keep the existing CUSTOM_HEADER_HTML behavior for reference.
fd48df5 to
856cb4a
Compare
It adds new options to customize the logo URL, the addon name, and the Discord URL.
Summary by CodeRabbit
New Features
Documentation / Logging
✏️ Tip: You can customize this high-level summary in your review settings.