mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 08:01:35 +00:00
fix: filter every declared array element, and lock a oneOf default to the shown branch
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
55156962d6
commit
d71cc28e5c
@@ -105,6 +105,10 @@
|
||||
|
||||
<div class="mt-3">
|
||||
{#if hasArgs}
|
||||
<!-- The one thing here that runs before Run: a `dynselect-`/`dynmultiselect-`
|
||||
argument makes DynamicInput execute that entrypoint on mount to fill its options,
|
||||
so opening this card runs it, and Cancel does not undo it. Everything else waits
|
||||
for the user, and a new surface added here must keep it that way. -->
|
||||
<SchemaForm
|
||||
bind:schema
|
||||
helperScript={{ source: 'deployed', path: runForm.path, runnable_kind: 'script' }}
|
||||
|
||||
@@ -114,6 +114,27 @@ describe('conformArgsToSchema', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// A list element is bound to a widget the same way a top-level argument is, so an
|
||||
// object in a scalar slot renders as [object Object] and the run carries it verbatim.
|
||||
it('filters array elements whose schema declares no properties', () => {
|
||||
expect(
|
||||
conformArgsToSchema(
|
||||
{ tags: ['ok', { token: '$var:u/ada/prod' }] },
|
||||
{ properties: { tags: { type: 'array', items: { type: 'string' } } } }
|
||||
)
|
||||
).toMatchObject({ args: { tags: ['ok'] }, dropped: { unshowable: ['tags[1]'] } })
|
||||
expect(
|
||||
conformArgsToSchema(
|
||||
{ rows: [{ n: 1, evil: 2 }] },
|
||||
{
|
||||
properties: {
|
||||
rows: { items: { oneOf: [{ title: 'a', properties: { n: { type: 'number' } } }] } }
|
||||
}
|
||||
}
|
||||
)
|
||||
).toMatchObject({ args: { rows: [{ n: 1 }] }, dropped: { undeclared: ['rows[0].evil'] } })
|
||||
})
|
||||
|
||||
// MultiSelect maps over the value while rendering, so a non-array here throws and
|
||||
// takes the whole form with it — the user cannot even cancel what they were shown.
|
||||
it('drops a non-array in a dyn-multiselect slot', () => {
|
||||
@@ -182,6 +203,34 @@ describe('enforceDisabledDefaults', () => {
|
||||
])
|
||||
})
|
||||
|
||||
// The tag picks the branch, as it does in ArgInput: a default read off the branch the
|
||||
// form never showed both overwrites the value and reports a reset nobody made.
|
||||
it('takes a disabled default two branches share from the one the form shows', () => {
|
||||
const locked = (def: string) => ({ type: 'string', default: def, disabled: true })
|
||||
const collide = {
|
||||
properties: {
|
||||
either: {
|
||||
oneOf: [
|
||||
{ title: 'a', properties: { mode: locked('alpha') } },
|
||||
{ title: 'b', properties: { mode: locked('beta') } }
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
expect(enforceDisabledDefaults({ either: { kind: 'a', mode: 'alpha' } }, collide)).toEqual({
|
||||
args: { either: { kind: 'a', mode: 'alpha' } },
|
||||
resetKeys: []
|
||||
})
|
||||
expect(enforceDisabledDefaults({ either: { kind: 'b', mode: 'x' } }, collide)).toEqual({
|
||||
args: { either: { kind: 'b', mode: 'beta' } },
|
||||
resetKeys: ['either.mode']
|
||||
})
|
||||
// Untagged opens the first branch, so that is the default it must enforce.
|
||||
expect(enforceDisabledDefaults({ either: { mode: 'x' } }, collide).args).toEqual({
|
||||
either: { mode: 'alpha' }
|
||||
})
|
||||
})
|
||||
|
||||
it('reports only the arguments it actually overwrote', () => {
|
||||
const { args, resetKeys } = enforceDisabledDefaults({ free: 'kept' }, schema)
|
||||
// The default still runs; the caller supplied nothing to overwrite, and one told
|
||||
|
||||
@@ -17,14 +17,20 @@ export function enforceDisabledDefaults(
|
||||
// it in place, and one branch returning the input would write through to their copy.
|
||||
if (!schema?.properties) return { args: { ...args }, resetKeys: [] }
|
||||
const resetKeys: string[] = []
|
||||
const result = mapMatchingArgs(args, schema.properties, isLockedProp, (value, prop, path) => {
|
||||
// An argument never supplied was not overwritten: the field shows the default
|
||||
// either way, and a caller told otherwise would try to correct what it never sent.
|
||||
// By value, since a default can be an object or an array: identity would report
|
||||
// every run of such a field as overridden, the caller that got it right included.
|
||||
if (value !== undefined && !deepEqual(value, prop.default)) resetKeys.push(path)
|
||||
return prop.default
|
||||
})
|
||||
const result = mapMatchingArgs(
|
||||
args,
|
||||
schema.properties,
|
||||
isLockedProp,
|
||||
(value, prop, path) => {
|
||||
// An argument never supplied was not overwritten: the field shows the default
|
||||
// either way, and a caller told otherwise would try to correct what it never sent.
|
||||
// By value, since a default can be an object or an array: identity would report
|
||||
// every run of such a field as overridden, the caller that got it right included.
|
||||
if (value !== undefined && !deepEqual(value, prop.default)) resetKeys.push(path)
|
||||
return prop.default
|
||||
},
|
||||
true
|
||||
)
|
||||
return { args: result, resetKeys }
|
||||
}
|
||||
|
||||
@@ -120,7 +126,9 @@ function dropUndeclaredNested(value: any, prop: any, dropped: DroppedPaths, path
|
||||
if (prop?.properties && !isArray) {
|
||||
return dropUndeclaredArgs(value, prop.properties, dropped, path)
|
||||
}
|
||||
if (prop?.items?.properties && isArray) {
|
||||
// Every declared element shape, not just an object one: the guards above are what drop
|
||||
// an object in a scalar slot, so an element reaches them only by recursing here.
|
||||
if (prop?.items && isArray) {
|
||||
const kept: any[] = []
|
||||
value.forEach((item: any, i: number) => {
|
||||
const itemPath = `${path}[${i}]`
|
||||
@@ -140,6 +148,13 @@ function dropUndeclaredNested(value: any, prop: any, dropped: DroppedPaths, path
|
||||
return value
|
||||
}
|
||||
|
||||
/** The `oneOf` branch the form shows: `ArgInput` opens the one the value's tag names,
|
||||
* and the first branch when no tag names any. */
|
||||
function selectedOneOfBranch(value: any, oneOf: any[]): any {
|
||||
const tag = ONE_OF_TAG_KEYS.map((key) => value?.[key]).find((v) => typeof v === 'string')
|
||||
return oneOf.find((branch) => tag != null && branch?.title === tag) ?? oneOf[0]
|
||||
}
|
||||
|
||||
/**
|
||||
* Rebuild `holder` with `visit` applied to every argument whose schema property matches
|
||||
* `isLeaf`, at any depth; returning `undefined` removes that argument.
|
||||
@@ -148,12 +163,16 @@ function dropUndeclaredNested(value: any, prop: any, dropped: DroppedPaths, path
|
||||
* property, for each element of an object-typed array, and for a `oneOf` branch. A
|
||||
* matching field mounts the same way down any of them, so a level left unvisited here is
|
||||
* one the model can prefill.
|
||||
*
|
||||
* Set `selectedBranchOnly` for a visitor whose result comes from the property it is
|
||||
* handed, so that a `oneOf` yields the branch the form shows rather than all of them.
|
||||
*/
|
||||
function mapMatchingArgs(
|
||||
holder: any,
|
||||
properties: Record<string, any>,
|
||||
isLeaf: (prop: any) => boolean,
|
||||
visit: (value: unknown, prop: any, path: string) => unknown,
|
||||
selectedBranchOnly = false,
|
||||
path = '',
|
||||
inOneOf = false
|
||||
): any {
|
||||
@@ -167,8 +186,8 @@ function mapMatchingArgs(
|
||||
// A matching object is a leaf, not a level: a password object is stored whole as a
|
||||
// single $jsonvar: reference, and a file is one opaque base64 string.
|
||||
if (isLeaf(prop)) {
|
||||
// Every oneOf branch is visited, so an absent argument under one belongs to a
|
||||
// variant that was not selected: a visitor that writes would add it to the run.
|
||||
// Reached only where every branch is visited, so an absent argument under one
|
||||
// belongs to a variant nobody selected: writing it would add it to the run.
|
||||
if (inOneOf && !Object.hasOwn(result, key)) continue
|
||||
const mapped = visit(result[key], prop, keyPath)
|
||||
if (mapped === undefined) delete result[key]
|
||||
@@ -179,24 +198,44 @@ function mapMatchingArgs(
|
||||
// leave the key behind holding undefined.
|
||||
if (!Object.hasOwn(result, key)) continue
|
||||
if (prop?.properties) {
|
||||
result[key] = mapMatchingArgs(result[key], prop.properties, isLeaf, visit, keyPath, inOneOf)
|
||||
result[key] = mapMatchingArgs(
|
||||
result[key],
|
||||
prop.properties,
|
||||
isLeaf,
|
||||
visit,
|
||||
selectedBranchOnly,
|
||||
keyPath,
|
||||
inOneOf
|
||||
)
|
||||
} else if (prop?.items?.properties && Array.isArray(result[key])) {
|
||||
result[key] = result[key].map((item: any, i: number) =>
|
||||
mapMatchingArgs(item, prop.items.properties, isLeaf, visit, `${keyPath}[${i}]`, inOneOf)
|
||||
mapMatchingArgs(
|
||||
item,
|
||||
prop.items.properties,
|
||||
isLeaf,
|
||||
visit,
|
||||
selectedBranchOnly,
|
||||
`${keyPath}[${i}]`,
|
||||
inOneOf
|
||||
)
|
||||
)
|
||||
} else if (Array.isArray(prop?.oneOf)) {
|
||||
// Every branch, not the one the value's tag names: which branch is selected is
|
||||
// runtime state, and a missing or stale tag must not decide whether an argument
|
||||
// is visited.
|
||||
for (const branch of prop.oneOf) {
|
||||
// One branch or all of them, and the difference is the visitor: a value can carry
|
||||
// a key from a variant nobody selected, so stripping has to reach every branch,
|
||||
// while a default read off an unselected branch is one the form never showed.
|
||||
const branches = selectedBranchOnly
|
||||
? [selectedOneOfBranch(result[key], prop.oneOf)]
|
||||
: prop.oneOf
|
||||
for (const branch of branches) {
|
||||
if (branch?.properties) {
|
||||
result[key] = mapMatchingArgs(
|
||||
result[key],
|
||||
branch.properties,
|
||||
isLeaf,
|
||||
visit,
|
||||
selectedBranchOnly,
|
||||
keyPath,
|
||||
true
|
||||
!selectedBranchOnly
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user