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.
5 tasks
lonix
added a commit
that referenced
this pull request
May 27, 2026
#444) (#469) * webui: operator-friendly schedule editor for cron-typed settings (closes #444) Three config keys store raw cron strings — voicetracking.announcements.schedule, voicetracking.cleanup.schedule, leaderboard_roles.update_cron — and since #439 (PR #452) they've been annotated as type: "cron" and rendered as plain text inputs awaiting this PR. Raw cron is power-user- only; the vast majority of real operator intents are expressible as (frequency, time-of-day, day). New picker UI: src/web/admin-views.ts: - New parseCronToPickerState() recognises the three supported shapes (daily `M H * * *`, weekly `M H * * DOW`, monthly `M H DOM * *`) and returns CronPickerState. Anything that doesn't cleanly match — step values, ranges, lists, named months, malformed input — falls back to `mode: "custom"` with the raw cron preserved verbatim so the operator's expression round-trips on save. - New renderCronPicker() emits a <div class="cron-picker"> containing: - the canonical hidden <input name="value"> the form actually submits, - a <select class="cron-mode"> with Daily / Weekly / Monthly / Custom, - a <input type="time">, - a weekday <select> (Sunday–Saturday, shown only for Weekly), - a day-of-month <input type="number"> (shown only for Monthly), - a raw cron <input type="text"> (shown only for Custom). Mode-specific wrappers carry the `hidden` attribute server-side; JS toggles them as the operator changes the mode. - renderSettingInput dispatches to renderCronPicker for r.type === "cron". src/web/admin-layout.ts: - New CRON_PICKER_SCRIPT wires up every .cron-picker on the page on load. On any control change it recomputes the cron string and writes it to the hidden value field, so form submission posts a canonical cron without any server-side parsing changes. data-mode attribute on the picker mirrors the current mode so CSS / tests can hook in. - Small CSS for the picker (inline-flex, gap, themed inputs). No backend changes: coerceConfigValue already accepts arbitrary strings for cron-typed keys (its category is `string` per defaultConfig). The picker JS turns its state into a cron string client-side; the server-side path is unchanged. Existing deployments' cron values are parsed back into picker state on first render (best-effort; falls back to custom for unrecognised shapes). Tests: - 4 parser tests covering daily / weekly (incl. Sunday-7 → Sunday-0 normalisation) / monthly / custom-fallback for step values, ranges, lists, named months, malformed input, and out-of-range fields. - 4 renderer tests covering each mode's pre-population from a stored cron, plus the data-mode attribute and the custom-mode raw value preservation. - Removed the old "placeholder until #444" assertion that wanted a text input — replaced by the picker. Out of scope: the wizard's per-feature step renderers (`renderWizardStepPage`) also accept cron values but currently render them as plain inputs. The wizard schema lookups go through the same metadata table; flipping its input rendering to use the picker is mechanical and can ride a later follow-up. * fix: address Copilot review on #469 Four valid points from review: 1. parseCronToPickerState now strips surrounding single/double quotes before splitting, matching the same normalisation the runtime services (voice-channel-truncation:225, voice-channel-announcer:110, scheduled-announcement-service:68) apply before handing the cron to CronJob. Without this a stored value like `"0 16 * * 5"` would have silently fallen back to custom mode even though the bot itself treats it as the unquoted form. 2. CRON_PICKER_SCRIPT now listens for both `input` and `change` on the picker controls. <select> elements (mode, dow) don't reliably fire `input` in some browsers, which could leave the hidden value field stale after a weekday selection. 3. compute() now clamps every numeric field to its valid cron range before assembling the string: hour 0–23, minute 0–59, dow 0–6, dom 1–31. The day-of-month <input> carries min/max but browsers only enforce that on submit (unevenly), so an operator typing 32 or 0 could otherwise produce an invalid cron that the bot then rejects and falls back to defaults for. 4. Dropped the unused `pad` helper from the script. Added one parser test exercising the new quote-stripping case for both single and double quotes. --------- Co-authored-by: Claude <noreply@anthropic.com>
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