fix(ui): render abstract wrapper correctly when selected as choice member#3202
fix(ui): render abstract wrapper correctly when selected as choice member#3202blut-agent wants to merge 2 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR fixes a bug where selecting an abstract-wrapper field through a choice wrapper caused the abstract wrapper to be skipped during node visualization. The implementation normalizes the effective wrapper spec to use the abstract-wrapper strategy when detected, and the test suite validates source-side, target-side, and expansion behavior for this scenario. ChangesChoice-selected abstract wrapper handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
| const candidateChildren = VisualizationService.generateNonDocumentNodeDataChildren(freshAbstractNode); | ||
| expect(candidateChildren.map((c) => c.title)).toEqual(['catName']); | ||
| }); | ||
| const candidateChildren = VisualizationService.generateNonDocumentNodeDataChildren(freshAbstractNode); |
There was a problem hiding this comment.
Why updating before the describe?
There was a problem hiding this comment.
Good question. This is a structural change to the test file.
The original describe('VisualizationService / abstract fields') block contained a single existing test. Since I added a new test suite for the choice-selected abstract wrapper feature, I closed the original describe block and created a new describe block for the new tests.
So what looks like 'updating' is really just:
- Closing the existing describe block with proper indentation
- Adding a new describe block with the new tests
The original test logic is unchanged — just properly closed out.
|
@PVinaches regarding the test ordering: the spy/mock is set up before the Also — the SonarCloud check has passed with 'Quality Gate passed'. The '1 new issue' flagged in the PR comments was a pre-existing coverage gap in |
3c02fcf to
ba6841f
Compare
…mber When a choice wrapper selects a member that is itself an abstract wrapper field, the visualization now delegates to the ABSTRACT_WRAPPER spec instead of CHOICE_WRAPPER. This creates an AbstractFieldNodeData (or TargetAbstractFieldNodeData) node, preserving the abstract badge and field-substitution context menu that were previously lost. Before this fix, selecting an abstract field inside a choice would create a ChoiceFieldNodeData for the abstract member, which prevented the abstract wrapper badge from rendering and blocked field substitution on the abstract element. Adds test cases for choice-selected abstract wrapper rendering in both source and target directions. Closes KaotoIO#3200
ba6841f to
4e74e53
Compare
|



Summary
When a choice wrapper (
xs:choice) selects a member that is itself an abstract wrapper field (xs:elementwith substitution group), the visualization now correctly delegates to theABSTRACT_WRAPPERspec instead ofCHOICE_WRAPPER. This creates anAbstractFieldNodeData(orTargetAbstractFieldNodeData) node, preserving the abstract badge and field-substitution context menu that were previously lost.The Problem
Before this fix, selecting an abstract field inside a choice would create a
ChoiceFieldNodeDatafor the abstract member. This caused:This is particularly visible in FIXML schemas where a
Messageabstract element appears as a choice member — selecting it would skip rendering the abstract wrapper entirely.The Fix
In
doGenerateNodeDataFromWrapperField, when the effective spec isCHOICE_WRAPPERbut the selected member haswrapperKind === 'abstract', we switch to usingABSTRACT_WRAPPERspec for node creation. This ensures:AbstractFieldNodeDatainstead ofChoiceFieldNodeDatasetWrapperRefTesting
Added 4 test cases covering:
AbstractFieldNodeDataTargetAbstractFieldNodeDataNote: Tests could not be run locally due to Jest running out of memory (OOM) on this large monorepo. The test patterns follow existing abstract test conventions in the same file. CI validation is recommended.
Related
Closes #3200
Summary by CodeRabbit