feat(console): dynamic form dropdowns via x-cozystack-options (DynamicOptionsWidget)#30
Merged
Aleksei Sviridkin (lexfrei) merged 11 commits intoJun 2, 2026
Merged
Conversation
Add a single generic DynamicOptionsWidget that renders any string field carrying the x-cozystack-options schema keyword as a <select>, populated from the cozystack-api Option resource (one source per dropdown). A shared addDynamicOptionWidgets walker binds the widget across nested properties, array items and additionalProperties (so vm-instance disks[].name and kubernetes nodeGroups.* are covered); the AdditionalPropertiesField passes it through to its nested forms. Replaces the bespoke StorageClassWidget, VMDiskWidget and BackupClassWidget (default-StorageClass preselect and disk-size labels are preserved via the server-provided item.default flag and labels), so every form dropdown now uses one mechanism. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
The standalone backup-resource create pages (Plan, BackupJob, Backup, RestoreJob) maintained a parallel client-side dropdown mechanism: they fetched BackupClasses / Plans / Backups / application kinds and injected them into the CRD schema as static enum arrays via enumMap. The CRD schemas now carry the x-cozystack-options keyword on these fields, so DynamicOptionsWidget renders them automatically from the cozystack-api Option resource. Drop the now-redundant enumMap entries and their backing useK8sList fetches for backupClassName, planRef.name, backupRef.name and applicationRef.kind / targetApplicationRef.kind. Keep applicationRef.name and targetApplicationRef.name on enumMap: they depend on the sibling kind field, a context the Option contract cannot express. Add unit coverage for the addDynamicOptionWidgets walker over the nested backup CRD shape. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
📝 WalkthroughWalkthroughThis PR replaces three field-name-based dropdown widgets with a single schema-keyword-driven ChangesWidget System Refactor: Schema-Keyword-Based Dynamic Options
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
area/console
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request consolidates several field-specific widgets (such as StorageClassWidget, VMDiskWidget, and BackupClassWidget) into a single, generic DynamicOptionsWidget driven by the x-cozystack-options schema keyword. It introduces utility functions to recursively apply this widget across forms and simplifies multiple creation pages by offloading option resolution to the cluster. The review feedback identifies a logic inversion bug in the auto-defaulting behavior of DynamicOptionsWidget that prevents users from clearing the field, suggests adding a free-text fallback input when no options are available, and recommends sanitizing the ui parameter in buildUi for improved robustness.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+75
to
+102
| return ( | ||
| <select | ||
| value={currentValue} | ||
| onChange={(e) => { | ||
| if (!e.target.value) hasAutoDefaulted.current = false | ||
| onChange(e.target.value || undefined) | ||
| }} | ||
| disabled={disabled || readonly} | ||
| required={required} | ||
| className="w-full rounded-lg border border-slate-300 bg-white pl-3 pr-8 py-2 text-sm text-slate-900 outline-none focus:border-blue-400 focus:ring-1 focus:ring-blue-400 disabled:opacity-50 disabled:cursor-not-allowed" | ||
| > | ||
| {/* Explicit placeholder so a value-less required select shows it instead | ||
| of silently displaying the first option. */} | ||
| <option value="" disabled={required}> | ||
| {placeholder} | ||
| </option> | ||
| {/* Keep the committed value visible even before the list loads, so an | ||
| async re-render never drops the parent's selection. */} | ||
| {currentValue && !hasCurrentInList && ( | ||
| <option value={currentValue}>{currentValue}</option> | ||
| )} | ||
| {items.map((it) => ( | ||
| <option key={it.value} value={it.value} title={it.description}> | ||
| {it.label || it.value} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| ) |
There was a problem hiding this comment.
This change addresses two key issues in the dropdown widget:
- Logic Inversion Bug in Auto-Defaulting: The current implementation resets
hasAutoDefaulted.current = falsewhen the user clears the field (i.e., when!e.target.valueis true). This causes theuseEffectto immediately re-trigger and re-apply the default value, making it impossible for the user to deliberately clear the field or select-- None --. Removing this reset ensures the default is only applied once on initial load. - Missing Free-Text Fallback: The PR description mentions a "free-text fallback when a source is unavailable". However, the widget currently only renders a
<select>element. If theOptionresource is missing or empty, the user is left with an empty dropdown and cannot proceed. Falling back to a standard<input type="text">when loading is complete anditemsis empty solves this issue.
if (!isLoading && items.length === 0) {
return (
<input
type="text"
value={currentValue}
onChange={(e) => onChange(e.target.value || undefined)}
disabled={disabled || readonly}
required={required}
placeholder={placeholder}
className="w-full rounded-lg border border-slate-300 bg-white px-3 py-2 text-sm text-slate-900 outline-none focus:border-blue-400 focus:ring-1 focus:ring-blue-400 disabled:opacity-50 disabled:cursor-not-allowed"
/>
)
}
return (
<select
value={currentValue}
onChange={(e) => onChange(e.target.value || undefined)}
disabled={disabled || readonly}
required={required}
className="w-full rounded-lg border border-slate-300 bg-white pl-3 pr-8 py-2 text-sm text-slate-900 outline-none focus:border-blue-400 focus:ring-1 focus:ring-blue-400 disabled:opacity-50 disabled:cursor-not-allowed"
>
{/* Explicit placeholder so a value-less required select shows it instead
of silently displaying the first option. */}
<option value="" disabled={required}>
{placeholder}
</option>
{/* Keep the committed value visible even before the list loads, so an
async re-render never drops the parent's selection. */}
{currentValue && !hasCurrentInList && (
<option value={currentValue}>{currentValue}</option>
)}
{items.map((it) => (
<option key={it.value} value={it.value} title={it.description}>
{it.label || it.value}
</option>
))}
</select>
)
Comment on lines
+14
to
+17
| function buildUi(node: unknown, ui: Record<string, unknown> = {}): Record<string, unknown> { | ||
| if (!node || typeof node !== "object") return ui | ||
| const n = node as Record<string, unknown> | ||
| const result = { ...ui } |
There was a problem hiding this comment.
To enforce defensive programming and ensure robustness, we should sanitize the ui parameter before spreading it. If ui is passed as a non-object (such as a string or null), { ...ui } can lead to unexpected behavior or runtime errors. Ensuring safeUi is a plain object prevents these issues.
Suggested change
| function buildUi(node: unknown, ui: Record<string, unknown> = {}): Record<string, unknown> { | |
| if (!node || typeof node !== "object") return ui | |
| const n = node as Record<string, unknown> | |
| const result = { ...ui } | |
| function buildUi(node: unknown, ui: unknown = {}): Record<string, unknown> { | |
| const safeUi = ui && typeof ui === "object" && !Array.isArray(ui) ? (ui as Record<string, unknown>) : {} | |
| if (!node || typeof node !== "object") return safeUi | |
| const n = node as Record<string, unknown> | |
| const result = { ...safeUi } |
The migration to the generic widget removed the StorageClass/VMDisk/BackupClass widget test suites, dropping coverage of behavior that absorbed several prior bug fixes. Pin it on DynamicOptionsWidget: auto-selecting the server-marked default once, showing an explicit placeholder for a required empty select instead of the first option, keeping a committed value visible while the list loads, emitting undefined (not "") on clear, and resolving items only from the Option whose name matches the field's source. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
…roperties branches The walker recurses into array items and additionalProperties maps — the whole reason it exists over inlining (vm-instance disks[].name, kubernetes nodeGroups.*.instanceType) — but only the properties path was tested. Add cases for both recursion branches. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
…down The applicationRef.kind field now resolves through the appkind option source, so it renders as a dropdown for every apiGroup — it is no longer gated to apps.cozystack.io. Only the applicationRef.name enum remains gated (it is served client-side and depends on the chosen kind). Update the comments that still described the old free-text-fallback behavior for the kind field. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
…eline The forms pipeline section still listed the removed StorageClass/BackupClass/ VMDisk field-name widgets and framed binding as a field-name convention. Update it to describe addDynamicOptionWidgets binding any field carrying x-cozystack-options to DynamicOptionsWidget, recursing into properties, array items and additionalProperties. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Two comments still pointed at entities this migration removed: the backup-class access hook claimed the dropdown is powered by a tenant list on backupclasses (it is now served by the Option resource), and the SchemaForm default-emit comment cited plansData/backupClassesData as the async-sibling example (both deleted). Correct both to match the code. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
The walker recurses into properties/items/additionalProperties but not the composition keywords, mirroring the same intentional limitation in addSensitiveStringWidgets. Pin it with a FIXME so the gap is documented rather than silent. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
The generic widget auto-selects only the server-marked default item, so a default-less source (vmdisk) must not auto-commit on load — the invariant the old VMDiskWidget enforced to avoid dropping the user's disk on a fast submit. Also pin that the Option resource is listed namespaced to the active tenant (querying it cluster-wide would return nothing). Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
…put uiSchema The pipeline contract (and the sibling sensitive-fields walker) requires the binders to return a new object and leave the input untouched. Pin it for addDynamicOptionWidgets: a nested input uiSchema stays byte-identical and the result's touched sub-objects are fresh references. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
…plying the default The select onChange reset the auto-default guard whenever the field was cleared, which re-armed the effect: for an optional field with a server-marked default, clearing emitted undefined, the effect then saw no value and re-selected the default, so the field could never be cleared. Latch the guard after the first auto-default and never reset it — clearing now sticks, matching the comment's stated intent. Required fields were unaffected (their empty option is disabled). Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Aleksei Sviridkin (lexfrei)
added
the
do-not-merge/hold
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/console/src/components/DynamicOptionsWidget.tsx`:
- Around line 62-68: The effect currently only sets hasAutoDefaulted inside the
branch that applies the default, so fields that mount with a pre-filled value
never arm the latch and will snap back if cleared later; change the useEffect
for hasAutoDefaulted so it arms as soon as loading settles: inside the effect,
if (!hasAutoDefaulted.current && !isLoading) set hasAutoDefaulted.current =
true, and separately (only when !value && defaultItem) call
onChange(defaultItem.value); keep the same dependencies and preserve the
existing guards for applying the default (value, defaultItem, isLoading,
onChange).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da084961-8f5d-4425-acbf-5ca5fadcfd1a
📒 Files selected for processing (19)
CLAUDE.mdapps/console/src/components/AdditionalPropertiesField.tsxapps/console/src/components/BackupClassWidget.test.tsxapps/console/src/components/BackupClassWidget.tsxapps/console/src/components/DynamicOptionsWidget.test.tsxapps/console/src/components/DynamicOptionsWidget.tsxapps/console/src/components/SchemaForm.tsxapps/console/src/components/StorageClassWidget.test.tsxapps/console/src/components/StorageClassWidget.tsxapps/console/src/components/VMDiskWidget.test.tsxapps/console/src/components/VMDiskWidget.tsxapps/console/src/components/rjsf-templates.tsxapps/console/src/hooks/useBackupClassAdminAccess.tsapps/console/src/lib/dynamic-options.test.tsapps/console/src/lib/dynamic-options.tsapps/console/src/routes/BackupCreatePage.tsxapps/console/src/routes/BackupJobCreatePage.tsxapps/console/src/routes/BackupPlanCreatePage.tsxapps/console/src/routes/BackupRestoreJobCreatePage.tsx
💤 Files with no reviewable changes (6)
- apps/console/src/components/StorageClassWidget.test.tsx
- apps/console/src/components/BackupClassWidget.test.tsx
- apps/console/src/components/VMDiskWidget.tsx
- apps/console/src/components/VMDiskWidget.test.tsx
- apps/console/src/components/BackupClassWidget.tsx
- apps/console/src/components/StorageClassWidget.tsx
Comment on lines
+62
to
+68
| const hasAutoDefaulted = useRef(false) | ||
| useEffect(() => { | ||
| if (!hasAutoDefaulted.current && !value && defaultItem && !isLoading) { | ||
| hasAutoDefaulted.current = true | ||
| onChange(defaultItem.value) | ||
| } | ||
| }, [value, defaultItem, isLoading, onChange]) |
There was a problem hiding this comment.
Auto-default re-arms when the field mounts with a pre-existing value.
The latch is only set inside the apply branch, so it stays false whenever the field mounts with a committed value (the !value guard skips). If the user later clears that field, the effect now sees !hasAutoDefaulted.current && !value && defaultItem && !isLoading and re-applies the default — making a pre-filled optional field impossible to clear. This is the exact snap-back the latch is meant to prevent, but it only holds for fields that start empty. Arm the latch once loading settles, independent of whether a default was applied:
🐛 Proposed fix
const hasAutoDefaulted = useRef(false)
useEffect(() => {
- if (!hasAutoDefaulted.current && !value && defaultItem && !isLoading) {
- hasAutoDefaulted.current = true
- onChange(defaultItem.value)
- }
+ if (hasAutoDefaulted.current || isLoading) return
+ // Latch on the first settled render so a later clear never re-arms the
+ // default, even when the field loaded with a pre-existing value.
+ hasAutoDefaulted.current = true
+ if (!value && defaultItem) onChange(defaultItem.value)
}, [value, defaultItem, isLoading, onChange])A matching test (mount with a value + server default, clear, assert no snap-back) would lock this in.
📝 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.
Suggested change
| const hasAutoDefaulted = useRef(false) | |
| useEffect(() => { | |
| if (!hasAutoDefaulted.current && !value && defaultItem && !isLoading) { | |
| hasAutoDefaulted.current = true | |
| onChange(defaultItem.value) | |
| } | |
| }, [value, defaultItem, isLoading, onChange]) | |
| const hasAutoDefaulted = useRef(false) | |
| useEffect(() => { | |
| if (hasAutoDefaulted.current || isLoading) return | |
| // Latch on the first settled render so a later clear never re-arms the | |
| // default, even when the field loaded with a pre-existing value. | |
| hasAutoDefaulted.current = true | |
| if (!value && defaultItem) onChange(defaultItem.value) | |
| }, [value, defaultItem, isLoading, onChange]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/console/src/components/DynamicOptionsWidget.tsx` around lines 62 - 68,
The effect currently only sets hasAutoDefaulted inside the branch that applies
the default, so fields that mount with a pre-filled value never arm the latch
and will snap back if cleared later; change the useEffect for hasAutoDefaulted
so it arms as soon as loading settles: inside the effect, if
(!hasAutoDefaulted.current && !isLoading) set hasAutoDefaulted.current = true,
and separately (only when !value && defaultItem) call
onChange(defaultItem.value); keep the same dependencies and preserve the
existing guards for applying the default (value, defaultItem, isLoading,
onChange).
Aleksei Sviridkin (lexfrei)
approved these changes
Jun 2, 2026
Contributor
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Generic DynamicOptionsWidget driven by the x-cozystack-options schema keyword, replacing the three field-name-bound widgets (StorageClass/BackupClass/VMDisk) and the per-page client-side enum builders. The backend contract it depends on has landed (Option resource + x-cozystack-options annotations), so the dropdowns are live rather than degrading to free-text. typecheck and the full vitest suite pass; behavior parity with the removed widgets is pinned by tests (auto-default-once, VMDisk no-auto-default, required placeholder, committed-value-visible-while-loading, clear-sticks-without-snapback, namespaced Option query), and the walker's properties/items/additionalProperties recursion, no-mutation invariant, and intentional oneOf/anyOf/allOf gap are all covered. Verified the integration with current main is clean (306 tests green on the merged tree). LGTM.
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
2 participants
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.
What
DynamicOptionsWidget+ aSchemaFormwalker (lib/dynamic-options.ts) that turns any schema field carryingx-cozystack-optionsinto a dropdown populated at runtime from theOptionresource (core.cozystack.io) in the current namespace, with free-text fallback when a source is unavailable.StorageClassWidget/VMDiskWidget/BackupClassWidgetwith the single generic mechanism.x-cozystack-options.Depends on
x-cozystack-optionsschemas).Screenshots
To add: dropdowns on VM/Kubernetes/VMDisk/Bucket/Backup create forms.
Summary by CodeRabbit