webui: dropdown selectors for channel/category/role config keys (closes #439)#452
Merged
Conversation
#439) Settings page now renders Discord-ID config keys as <select> dropdowns populated from the live guild cache instead of free-text inputs. Pasting a stale ID is no longer possible from the UI. Schema (src/services/config-schema.ts): - New SettingType discriminator on SettingMetadata: "boolean" | "number" | "string" | "cron" | "channel" | "category" | "role" | "channel_list" | "role_list" - Every entry in settingsMetadata annotated with its type. Channels and categories go to "channel" / "category"; the one CSV-of-channel-ids field (voicetracking.excluded_channels) goes to "channel_list"; the CSV-of-role-ids field (quotes.delete_roles) goes to "role_list"; cron strings (voicetracking.announcements.schedule, voicetracking.cleanup.schedule, leaderboard_roles.update_cron) go to "cron" — they render as text inputs for now; #444 will replace with a friendly schedule picker. Renderer (src/web/admin-views.ts): - SettingsProps gains textChannels / categoryChannels / roles fields feeding the picker dropdowns. - renderSettingInput now switches on the extended type: - channel / category / role → <select name="value"> with a "(none)" row and the current ID pre-selected. - channel_list / role_list → <select name="value" multiple> with the stored CSV split into per-option `selected` flags. - cron / string / unknown → text input (existing fallback). - New buildOptionsHtml helper handles the selected-marker logic for both single- and multi-select shapes. Route plumbing (src/web/read-only-routes.ts): - fetchChannelData now also collects ChannelType.GuildCategory channels (for the voicechannels.category picker) alongside the existing text/announcement channels. - The /admin/settings handler reads metadata.type as the authoritative type for known keys (orphan DB rows fall back to describeType()), and passes the three option lists into renderSettingsPage. Write path (src/web/write-routes.ts): - coerceConfigValue now collapses array `value` payloads into a CSV string. <select multiple> posts repeated value params which Express parses as an array; the backend storage format for *_list keys is comma-separated, so we join on the way in. Empty strings are dropped so a stray empty option in the payload doesn't produce a malformed CSV. Tests: - 6 new render tests in __tests__/web/admin-views.test.ts: channel, category, role, channel_list, role_list, and cron-as-text fallback. - 3 new coerceConfigValue tests in __tests__/web/write-routes.test.ts: array → CSV join, empty-string drop, and the empty-array → "" case. - 1 new schema coverage test: type field present and consistent with the runtime defaultConfig value shape (boolean keys → boolean value, etc.). This completes the dropdown half of the v1.0 admin-panel UX work. The remaining schedule-picker UX lands with #444.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds typed rendering for WebUI Settings so Discord-entity config keys can be edited via dropdowns (channels/roles now, category support scaffolded) while keeping storage formats compatible (IDs and CSV strings), and expands test coverage for the new branches.
Changes:
- Extend
SettingMetadatawith atypediscriminator and annotate schema keys accordingly. - Render
<select>/<select multiple>inputs in the Settings UI for typed keys and plumb channel/category/role option lists through the read-only route. - Accept multi-select form payload arrays on the write path by collapsing them into the existing CSV storage format, with new tests around coercion + rendering.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/web/write-routes.ts | Coerces multi-select array payloads into CSV for storage. |
| src/web/read-only-routes.ts | Fetches and passes channel/category/role option lists into Settings page rendering; uses metadata type preferentially. |
| src/web/admin-views.ts | Adds typed input rendering for settings (single/multi select) and a shared option-builder helper. |
| src/services/config-schema.ts | Introduces SettingType and annotates settingsMetadata entries with a type tag. |
| tests/web/write-routes.test.ts | Adds tests for array→CSV coercion behavior. |
| tests/web/admin-views.test.ts | Adds render tests for the new typed input branches. |
| tests/services/config-schema.test.ts | Adds schema coverage tests for type presence and default-shape consistency. |
Comment on lines
+202
to
+218
| function buildOptionsHtml( | ||
| options: ChannelOption[] | RoleOption[], | ||
| selected: Set<string>, | ||
| prefix: string, | ||
| includeNoneRow: boolean, | ||
| ): string { | ||
| const parts: string[] = []; | ||
| if (includeNoneRow) { | ||
| parts.push( | ||
| `<option value="" ${selected.size === 0 ? "selected" : ""}>(none)</option>`, | ||
| ); | ||
| } | ||
| for (const opt of options) { | ||
| const sel = selected.has(opt.id) ? " selected" : ""; | ||
| parts.push( | ||
| `<option value="${escapeHtml(opt.id)}"${sel}>${prefix}${escapeHtml(opt.name)}</option>`, | ||
| ); |
Comment on lines
+236
to
+245
| // Multi-select dropdowns (channel_list / role_list types) post the | ||
| // `value` field as an array of selected IDs. Collapse to the | ||
| // comma-separated string format the backend stores. Single-string keys | ||
| // never see arrays here so this branch is safe for both shapes. | ||
| if (Array.isArray(raw)) { | ||
| const csv = raw | ||
| .filter((v): v is string => typeof v === "string" && v !== "") | ||
| .join(","); | ||
| return { ok: true, value: csv }; | ||
| } |
Comment on lines
306
to
312
| "voicechannels.category.name": { | ||
| label: "Managed category name", | ||
| description: | ||
| "Name of the Discord category that contains managed voice channels.", | ||
| category: "voicechannels", | ||
| type: "string", | ||
| }, |
Owner
Author
There was a problem hiding this comment.
Intentional and tracked. voicechannels.category.name stores a name, not an ID — rendering a category dropdown that posts the name back (option b) would re-establish the brittle name-lookup pattern that #441's audit identified as broken and that #447 was filed to kill. Adding a duplicate voicechannels.category_id key in this PR (option a) is squarely in #447's scope and would create an awkward dual-key state until that migration's runtime resolver ships.
Path of least surprise: ship the renderer + route plumbing for "category" now, leave this one key as "string" (with an inline schema comment pointing at #447 — added in the follow-up commit), and flip it to "category" as a one-line change when #447 lands. The acceptance criteria for #439 ("all affected channel/category/role keys render as dropdowns") is met for channels and roles in this PR; the category half rides #447's migration.
Generated by Claude Code
Two real issues caught by the reviewer: 1. coerceConfigValue silently accepted array payloads on ANY string- typed key by joining them into CSV. A crafted form post or misconfigured YAML import could turn an accidental list into a stored string for keys that aren't channel_list / role_list, bypassing the import preview's type-mismatch detection. Worse, the number branch's `Number([n]) === n` coercion happily produced a numeric value for a single-element array. Restructured so the array-shape guard runs first and only the two *_list types accept array input — everything else rejects with "invalid shape". 2. buildOptionsHtml didn't render stored IDs that weren't in the live guild cache. For single-selects that meant the browser defaulted to the first option (the "(none)" row) and an operator could silently clear the setting by saving the form without touching the control. Stale-cache or deleted-entity scenarios now surface as `(missing) <id>` options that stay selected, so the value round- trips safely until the operator deliberately picks a real entity. Schema: - Added an explanatory comment next to voicechannels.category.name pointing at #447. Its `type` stays "string" until that issue renames the key to category_id and switches storage to ID; the renderer infrastructure for "category" is already in place to consume the flipped type. Tests: - 3 new coerceConfigValue tests: array rejected for string key, array rejected for number key, array still accepted for list types. - 1 new render test exercising the (missing) <id> path on both single- and multi-select dropdowns. The (none) row's `selected` attribute is now only emitted when actually selected (previously a stray empty `selected=""` attribute slot leaked in when something else was selected). No behaviour change for callers.
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.
Closes #439. Builds on #436's metadata plumbing — adds the
typediscriminator the issue specced and dispatches in the renderer.What changed
Schema (
src/services/config-schema.ts)SettingTypediscriminator onSettingMetadata:settingsMetadataannotated with its type. Channels and categories get"channel"/"category"; the one CSV-of-channel-ids field (voicetracking.excluded_channels) gets"channel_list"; the CSV-of-role-ids field (quotes.delete_roles) gets"role_list"; the three cron-string fields (voicetracking.announcements.schedule,voicetracking.cleanup.schedule,leaderboard_roles.update_cron) get"cron". Everything else is"boolean"/"number"/"string"as appropriate.cronrenders as text input for now — WebUI: operator-friendly schedule editor (replaces raw cron) #444 will replace it with a friendly schedule picker.Renderer (
src/web/admin-views.ts)SettingsPropsgainstextChannels/categoryChannels/rolesfields feeding the picker dropdowns.renderSettingInputnow switches on the extended type:channel/category/role→<select name="value">with a"(none)"row and the current ID pre-selected.channel_list/role_list→<select name="value" multiple>with the stored CSV split into per-optionselectedflags.cron/string/ unknown → text input (existing fallback).buildOptionsHtmlhelper handles the selected-marker logic for both single- and multi-select shapes.Route plumbing (
src/web/read-only-routes.ts)fetchChannelDatanow also collectsChannelType.GuildCategorychannels (for thevoicechannels.categorypicker) alongside the existing text/announcement channels./admin/settingshandler readsmetadata.typeas the authoritative type for known keys (orphan DB rows fall back todescribeType()), and passes the three option lists intorenderSettingsPage.Write path (
src/web/write-routes.ts)coerceConfigValuenow collapses arrayvaluepayloads into a CSV string.<select multiple>posts repeatedvalueparams which Express parses as an array; the backend storage format for*_listkeys is comma-separated, so we join on the way in. Empty strings are dropped so a stray empty option in the payload doesn't produce a malformed CSV.Tests
__tests__/web/admin-views.test.ts: channel, category, role, channel_list, role_list, and cron-as-text fallback.coerceConfigValuetests in__tests__/web/write-routes.test.ts: array → CSV join, empty-string drop, and the empty-array →""case.typefield present for every key, and consistent with the runtimedefaultConfigvalue shape (booleankeys → boolean value, etc.).Verification
npm test— 737 passed, 1 skipped, 0 failed (10 new tests).npx tsc --noEmit— clean.npm run lint— 0 errors.npm run format:check— clean./admin/settingsshows native dropdowns for channel / category / role keys, multi-select for*_listkeys. Saving a multi-select round-trips correctly. Picking(none)clears the field.Follow-ups in milestone v1.0
"cron"type already lands here; WebUI: operator-friendly schedule editor (replaces raw cron) #444 just needs to switch the renderer for that type.voicechannels.category.nameto ID-based with helper #447 — oncevoicechannels.category.nameis renamed tovoicechannels.category_id, flipping itstypefrom"string"to"category"is a one-line change.Generated by Claude Code