feat: add dedicated Discord settings section to admin page#116
Merged
Conversation
Replace the raw "Other" section text input for enable_discord with a proper Discord section featuring a boolean toggle and read-only server config display (Client ID, Client Secret, Bot Token, Guild ID, Admin IDs, Geofence Forum Channel) with masked sensitive values.
hokiepokedad2
commented
Apr 5, 2026
Contributor
Author
hokiepokedad2
left a comment
There was a problem hiding this comment.
Code Review — PR #116
Grade: A | Verdict: APPROVED
| Metric | Rating |
|---|---|
| Code Quality | Excellent |
| Requirements Satisfied | All 7/7 |
| Architecture Fit | Consistent |
| Risk Level | Low |
| Test Coverage | Good (2 new tests, 562+461 pass) |
Requirements Traceability
| Requirement (from #115) | Status |
|---|---|
| Discord out of "Other" section → dedicated section | ✅ |
enable_discord as toggle (not text input) |
✅ |
| Match Telegram section pattern | ✅ |
| Show Client ID, Client Secret, Bot Token, Guild ID, Admin IDs, Forum Channel | ✅ All 6 shown |
Security Review
- ✅ Secrets masked server-side (
MaskSecret— last 4 chars only for Client Secret & Bot Token) - ✅ IDs partially masked (
MaskValue— first 4 + last 4 chars) - ✅ Endpoint is admin-only (403 for non-admins, tested)
- ✅ No write endpoint for server config —
.env+ restart required - ✅
discord_client_secretanddiscord_bot_tokenadded toSensitiveKeysfilter - ✅ Full secret values verified absent from serialized response (test assertion)
Code Quality Notes
- Clean hybrid design: editable
enable_discordtoggle (DB-backed) + read-only server config (IOptions) - Discord group follows identical
SETTING_GROUPSpattern as Telegram - Server config sub-section is a new UI pattern but cleanly contained
visibleGroupscorrectly extended to show Discord even beforeenable_discordexists in DB- Masking helpers handle null/empty/short values correctly
- SCSS uses existing CSS variables and responsive breakpoints consistently
Minor Observations (Non-blocking)
getDiscordConfig()inngOnInithas no error callback — acceptable since the section simply won't render on failure, but aconsole.warncould aid debugging- Group identification via string
'Discord'in template — fine for a single case
Test Coverage
GetDiscordConfigReturnsOkForAdmin— verifies response + secret maskingGetDiscordConfigReturnsForbidForNonAdmin— verifies access control- All 562 backend + 461 frontend tests pass, ESLint/Prettier/dotnet-format clean
No critical or major issues found. Clean, well-scoped implementation.
Telegram bot username and Discord server config are now conditionally shown only when the respective enable toggle is on.
Add masked Bot Token and Bot Username read-only fields to the Telegram section, matching the Discord server config pattern.
applyChange now inserts the setting into the settings array if it doesn't exist yet, matching the pattern already used by selectRepo. Fixes server config not appearing after enabling Discord/Telegram when the setting hasn't been saved to the DB before.
The enable_discord and enable_telegram toggles are not wired up yet (see #117), so hiding sections based on them is premature.
This file contains hidden or 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
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.
Summary
Closes #115
enable_discordboolean toggle (replacing the raw text input in the "Other" section)GET /api/settings/discord-configendpointenable_discordto the settings migration category map and boolean keysdiscord_client_secretanddiscord_bot_tokento the sensitive keys filterSecurity
.env+ restartSensitiveKeysset to prevent accidental exposure viaGET /api/settingsTest plan
GET /api/settings/discord-configenable_discordsetting doesn't exist in DB yet