webui: operator-friendly schedule editor for cron-typed settings (closes #444)#469
Merged
Conversation
#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.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an operator-friendly cron schedule editor to the WebUI Settings page by replacing raw cron text inputs for type: "cron" settings with a small picker UI (daily/weekly/monthly/custom) that still submits a canonical cron string to the existing backend write path.
Changes:
- Added cron parsing (
parseCronToPickerState) to infer picker state from stored 5-field cron strings (daily/weekly/monthly) with a safe custom fallback. - Rendered a cron picker control on
/admin/settingsfor cron-typed settings, preserving raw cron in custom mode. - Injected client-side wiring + minimal CSS into the admin layout to keep the hidden submitted value in sync with picker controls; added tests for parser + renderer.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/web/admin-views.ts |
Adds cron parser + cron picker renderer; switches type: "cron" settings to use the picker. |
src/web/admin-layout.ts |
Adds CSS and an inline script to wire cron picker controls and update the hidden submitted cron value. |
__tests__/web/admin-views.test.ts |
Replaces the old cron-text-placeholder test with new parser and cron-picker rendering tests. |
| dayOfMonth: 1, | ||
| raw: cron, | ||
| }; | ||
| const fields = cron.trim().split(/\s+/); |
| "show(customWrap,m==='custom');" + | ||
| "hidden.value=compute();p.setAttribute('data-mode',m)}" + | ||
| "mode.addEventListener('change',refresh);" + | ||
| "[time,dow,dom,custom].forEach(function(el){if(el)el.addEventListener('input',function(){hidden.value=compute()})});" + |
Comment on lines
+263
to
+267
| "var parts=(time.value||'00:00').split(':');" + | ||
| "var h=parseInt(parts[0],10)||0;var mi=parseInt(parts[1],10)||0;" + | ||
| "if(m==='daily')return mi+' '+h+' * * *';" + | ||
| "if(m==='weekly')return mi+' '+h+' * * '+dow.value;" + | ||
| "if(m==='monthly')return mi+' '+h+' '+(dom.value||'1')+' * *';" + |
Comment on lines
+249
to
+250
| "(function(){function pad(n){return String(n).padStart(2,'0')}" + | ||
| "function wire(p){var hidden=p.querySelector('input[name=value]');" + |
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.
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 #444. Builds on #439's
crontype discriminator.What changed
Three config keys (
voicetracking.announcements.schedule,voicetracking.cleanup.schedule,leaderboard_roles.update_cron) store raw cron strings. Since #439 (PR #452) they've been annotatedtype: "cron"and rendered as plain text inputs awaiting this PR. Raw cron is power-user-only — most operator intents are expressible as (frequency, time-of-day, day).Parser (
parseCronToPickerState)Recognises three shapes and returns picker state:
M H * * *0 0 * * *→ midnight every dayM H * * DOW0 16 * * 5→ Friday 16:00M H DOM * *0 0 1 * *→ first of every monthAnything that doesn't cleanly match — step values (
*/15), ranges (9-17), lists (1,15), named months (JAN), malformed input, out-of-range fields — falls back tomode: "custom"with the raw cron preserved verbatim so the operator's expression round-trips on save.7for Sunday is normalised to0.Renderer (
renderCronPicker)Emits a
<div class="cron-picker">containing:<input name="value">the form actually submits.<select class="cron-mode">— Daily / Weekly / Monthly / Custom.<input type="time">.<select>(Sunday–Saturday; shown only for Weekly).<input type="number">(shown only for Monthly).<input type="text">(shown only for Custom).Mode-specific wrappers carry the
hiddenattribute server-side; JS toggles them as the operator changes the mode. Thedata-modeattribute on the picker mirrors current state for CSS/tests.Client-side JS (
CRON_PICKER_SCRIPTinadmin-layout.ts)Wires up every
.cron-pickeron the page. On any control change it recomputes the cron string and writes it to the hiddenvaluefield. Form submission therefore posts a canonical cron — no backend changes.coerceConfigValuealready accepts arbitrary strings for cron-typed keys.CSS
Small additions for the picker:
inline-flex, themed inputs, sane defaults.Tests
data-modeattribute, and custom-mode raw value preservation.Out of scope
The wizard's per-feature step renderers (
renderWizardStepPage) also handle cron-valued keys but render them as plain inputs. Flipping that to use the picker is mechanical (the metadata'stypefield is already there); leaving it for a follow-up to keep this PR focused on the Settings page.Verification
npm test— 751 passed, 1 skipped, 0 failed (+8 new tests).npx tsc --noEmit— clean.npm run lint— 0 errors.npm run format:check— clean./admin/settingsshows a picker (not a text input) for each of the three cron-typed keys. Round-trip via Save reproduces the picker state. Picking "Custom" reveals a raw cron field.Generated by Claude Code