[orchestrator] Backport orchestrator fixes and enhancements to 1.9 #2669
Merged
Conversation
…2666) * feat(orchestrator): pre-populate Execute Workflow form from URL query params (#2570) * prepopulate workflow execution page form from URL query params Signed-off-by: Karthik <karthik.jk11@gmail.com> * support enum coercison for case-insenstive match and skip invalid values * add support for fields that are defined via '$ref' * add full support for json schema fields --------- Signed-off-by: Karthik <karthik.jk11@gmail.com> * fix(orchestrator-form-react): scope async validation to active step (#2602) * fix(orchestrator-form-react): scope async validation to active step Limit validate:url requests to the active step during multi-step navigation. Made-with: Cursor * fix(orchestrator-form-react): keep full formData for async validation Pass full formData to template evaluation while scoping uiSchema traversal. Made-with: Cursor * fix(orchestrator-form-react): preserve ui:hidden on objects with properties (#2653) * fix(orchestrator): honor json schema defaults in initial formData (#2654) * fix(orchestrator): honor json schema defaults Ensure extractStaticDefaults falls back to JSON Schema defaults when ui:props fetch:response:default is absent so initial formData includes schema defaults. Made-with: Cursor * chore(changeset): document schema default fix Add changeset for orchestrator form defaults update. Made-with: Cursor * fix(orchestrator): handle root defaults Avoid setting an empty key for root schema defaults and add tests to cover root default handling. Made-with: Cursor * fix(orchestrator): document and test defaults Clarify extractStaticDefaults precedence in docs and add test coverage for default handling. Made-with: Cursor * chore(orchestrator): update yarn.lock after backport Made-with: Cursor --------- Signed-off-by: Karthik <karthik.jk11@gmail.com> Co-authored-by: Karthik Jeeyar <karthik@redhat.com>
karthikjeeyar
requested review from
batzionb,
gciavarrini and
mareklibra
as code owners
Review Summary by QodoBackport orchestrator fixes and enhancements to 1.9
WalkthroughsDescription• Pre-populate Execute Workflow form from URL query parameters with full schema support • Scope async validation to active step in multi-step forms • Honor JSON Schema defaults in initial form data • Preserve ui:hidden on object properties with nested fields Diagramflowchart LR
A["URL Query Parameters"] -->|"mergeQueryParamsIntoFormData"| B["Form Data"]
C["JSON Schema"] -->|"extractStaticDefaults"| B
B -->|"generateUiSchema"| D["UI Schema"]
D -->|"preserves ui:hidden"| E["Rendered Form"]
F["Multi-step Form"] -->|"scoped validation"| G["Active Step Only"]
File Changes1. workspaces/orchestrator/plugins/orchestrator/src/components/ExecuteWorkflowPage/queryParamsToFormData.ts
|
Code Review by Qodo
1. allOf query prefill skipped
|
|
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
Comment on lines
+59
to
+86
| function pathExistsInSchema( | ||
| schema: JSONSchema7, | ||
| path: string, | ||
| root: JSONSchema7, | ||
| ): boolean { | ||
| if (!path) return true; | ||
| let s: JSONSchema7 | undefined = schema; | ||
| const parts = path.split('.'); | ||
|
|
||
| for (let i = 0; i < parts.length; i++) { | ||
| const part = parts[i]; | ||
| if (!s || typeof s === 'boolean') return false; | ||
| if (s.$ref) { | ||
| s = resolveRef(root, s.$ref); | ||
| i--; | ||
| continue; | ||
| } | ||
| if (s.oneOf || s.anyOf) { | ||
| const remaining = parts.slice(i).join('.'); | ||
| return pathExistsInComposite(s, remaining, root); | ||
| } | ||
| const props = s.properties; | ||
| if (!props || typeof props !== 'object') return false; | ||
| if (!(part in props)) return false; | ||
| s = props[part] as JSONSchema7; | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
1. Allof query prefill skipped 🐞 Bug ≡ Correctness
mergeQueryParamsIntoFormData can never prepopulate fields defined under an allOf-only schema node because pathExistsInSchema only branches for oneOf/anyOf and otherwise requires properties. This causes valid draft-07 schemas composed via allOf to be ignored during URL prepopulation despite the documented “full JSON Schema draft-07” support.
Agent Prompt
### Issue description
`mergeQueryParamsIntoFormData` skips query params for schemas that use `allOf` composition without inline `properties` because `pathExistsInSchema` only treats `oneOf/anyOf` as composites and otherwise requires `properties`.
### Issue Context
The implementation already uses `json-schema-library` (`getSchemaAtPath`) which can resolve `allOf` (and other composition/conditionals) using the proposed data. The manual `pathExistsInSchema` pre-check is therefore both redundant and currently incorrect for `allOf`-only composition.
### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator/src/components/ExecuteWorkflowPage/queryParamsToFormData.ts[59-109]
- workspaces/orchestrator/plugins/orchestrator/src/components/ExecuteWorkflowPage/queryParamsToFormData.ts[272-299]
### Recommended fix
Either:
1) Update `pathExistsInSchema` to treat `allOf` the same way as `oneOf/anyOf` (call `pathExistsInComposite` when `s.allOf` is present), and consider adding coverage for other composition-only nodes if you want the pre-check to match the “draft-07” claim.
OR (simpler and less error-prone):
2) Remove the `pathExistsInSchema(...)` gate and rely on `getSchemaAtPath(...)` returning `undefined` for unknown paths.
### Tests
Add a unit test showing that a schema like:
```ts
{ type: 'object', allOf: [{ type: 'object', properties: { language: { type: 'string' } } }] }
```
correctly prepopulates `?language=English`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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.
Hey, I just made a Pull Request!
Cherrypick of below PR's
#2654
#2602
#2653
#2570
Summary