fix(ui): ensure oneOf choices inherit type and properties for forms#3535
fix(ui): ensure oneOf choices inherit type and properties for forms#3535QuantumBreakz wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthrough
ChangesOneOf schema normalization
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
dd649ef to
cf1928b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/ui/src/services/document/xml-schema/xml-schema-document.service.test.ts (1)
1192-1193: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRun Prettier on the added test block.
Remove the trailing whitespace and format the callback consistently.
Proposed formatting
- - const orderElement = rootElement.fields.find(f => f.name === 'Order'); + + const orderElement = rootElement.fields.find((f) => f.name === 'Order');As per coding guidelines,
packages/ui/**/*.{ts,tsx}must use Prettier formatting.🤖 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 `@packages/ui/src/services/document/xml-schema/xml-schema-document.service.test.ts` around lines 1192 - 1193, Format the added test block according to Prettier, specifically removing trailing whitespace and applying consistent formatting to the callback used in the rootElement.fields.find expression.Source: Coding guidelines
packages/ui/src/utils/normalize-oneof-choices.test.ts (1)
10-55: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a regression test for recursive schemas.
The current fixture only exercises
anyOf→oneOfwhereparentPropertiesis available, so it does not detect the discarded-clone bug. Add a case withoneOfnested underpropertiesoritems, and assert both normalization and preservation of the original input schema.🤖 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 `@packages/ui/src/utils/normalize-oneof-choices.test.ts` around lines 10 - 55, Extend the test in normalizeOneOfChoices with a recursive schema containing oneOf nested under a property or items, where parentProperties is unavailable. Assert the nested choices are normalized as expected and verify the original schema remains unchanged after normalization, covering the discarded-clone regression.
🤖 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 `@packages/ui/src/services/document/xml-schema/xml-schema-document.service.ts`:
- Around line 514-529: Update the reachability traversal around the queue in the
filterUnreachable logic to treat xs:include relationships bidirectionally: from
each schema, enqueue included schemas and the schemas that include it, while
excluding xs:import relationships. Preserve the existing reachableSchemas
tracking and add a regression covering a root in one included file referencing a
type in a sibling included file.
In `@packages/ui/src/utils/normalize-oneof-choices.ts`:
- Around line 47-58: Update normalizeOneOfChoices so recursive calls in the
properties and generic object branches preserve and assign each returned
normalized child schema instead of discarding it. Ensure nested
oneOf/anyOf/allOf structures under properties, items, or combinators are
replaced with the normalized clones while retaining the existing
currentProperties behavior.
- Around line 27-31: Update the required-property handling in the loop over
choice.required to check currentProperties with
Object.prototype.hasOwnProperty.call before reading or copying values,
preventing inherited keys from being accepted. Ensure choice.properties is a
null-prototype object or copy properties via Object.defineProperty so keys such
as __proto__ cannot trigger the object prototype setter, while preserving title
propagation.
---
Nitpick comments:
In
`@packages/ui/src/services/document/xml-schema/xml-schema-document.service.test.ts`:
- Around line 1192-1193: Format the added test block according to Prettier,
specifically removing trailing whitespace and applying consistent formatting to
the callback used in the rootElement.fields.find expression.
In `@packages/ui/src/utils/normalize-oneof-choices.test.ts`:
- Around line 10-55: Extend the test in normalizeOneOfChoices with a recursive
schema containing oneOf nested under a property or items, where parentProperties
is unavailable. Assert the nested choices are normalized as expected and verify
the original schema remains unchanged after normalization, covering the
discarded-clone regression.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7098c8d7-4bb9-45bd-aa5d-b37692613a6b
📒 Files selected for processing (6)
packages/ui/src/components/Visualization/Canvas/Form/CanvasFormBody.tsxpackages/ui/src/services/document/xml-schema/xml-schema-document.service.test.tspackages/ui/src/services/document/xml-schema/xml-schema-document.service.tspackages/ui/src/utils/index.tspackages/ui/src/utils/normalize-oneof-choices.test.tspackages/ui/src/utils/normalize-oneof-choices.ts
| if (filterUnreachable && document.rootElement) { | ||
| const parentSchema = document.rootElement.getParent(); | ||
| const reachableSchemas = new Set<XmlSchema>(); | ||
| const queue: XmlSchema[] = [parentSchema]; | ||
|
|
||
| while (queue.length > 0) { | ||
| const current = queue.shift()!; | ||
| if (!reachableSchemas.has(current)) { | ||
| reachableSchemas.add(current); | ||
| const externals = current.getExternals(); | ||
| for (const ext of externals) { | ||
| const schema = ext.getSchema(); | ||
| if (schema) { | ||
| queue.push(schema); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Traverse xs:include relationships in both directions.
When the selected root is declared in an included schema, this only follows that schema’s outgoing references. Sibling schemas included by the same parent are filtered out, so referenced types can retain namedTypeFragmentRefs with no fragment. Treat xs:include edges as bidirectional (but not xs:import) and add a regression with a root in one included file referencing a type in another.
🤖 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 `@packages/ui/src/services/document/xml-schema/xml-schema-document.service.ts`
around lines 514 - 529, Update the reachability traversal around the queue in
the filterUnreachable logic to treat xs:include relationships bidirectionally:
from each schema, enqueue included schemas and the schemas that include it,
while excluding xs:import relationships. Preserve the existing reachableSchemas
tracking and add a regression covering a root in one included file referencing a
type in a sibling included file.
…aotoIO#3508) Problem: The Kaoto form engine (uniforms via `@kaoto/forms`) relies on each choice in a `oneOf` JSON schema array having `type: 'object'` and its own `properties` structure. When a `oneOf` schema choice only defines constraints (e.g. `{"required": ["data"]}`) on properties defined in the parent object, the form engine cannot render an appropriate choice option for it, leading to visual bugs and empty form sections. Solution: Created a `normalizeOneOfChoices` utility that intercepts JSON schemas before they are handed to `KaotoForm`. When traversing the schema, if a `oneOf` choice contains constraints but no type/properties, it now implicitly assumes `type: 'object'` and inherits the corresponding property definitions (and titles) from the parent schema. Fixes KaotoIO#3508
1fc5a14 to
1bcbb8a
Compare
|
|
Hello! Thanks for raising this PR, it is a very interesting solution. Have you checked if it could be solved directly from camel-catalog repo (here)? |
2 participants
Fixes #3508
Problem:
The Kaoto form engine (
@kaoto/forms) relies on each choice in aoneOfJSON schema array havingtype: 'object'and its ownpropertiesstructure. When aoneOfschema choice only defines constraints (e.g.{"required": ["data"]}) on properties defined in the parent object, the form engine cannot render an appropriate choice option for it.Solution:
Created a
normalizeOneOfChoicesutility that intercepts JSON schemas before they are handed toKaotoForm. When traversing the schema, if aoneOfchoice contains constraints but no type/properties, it now implicitly assumestype: 'object'and inherits the corresponding property definitions (and titles) from the parent schema.Summary by CodeRabbit
Bug Fixes
Tests