-
Notifications
You must be signed in to change notification settings - Fork 59
[Data Mapper - Inline] Fix undo button not working in data mapper #1014
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThese changes fix undo button behavior in the inline data mapper by resetting undo/redo state during initialization and preventing navigation away from the inline mapper context when undoing with no current artifact. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
workspaces/ballerina/ballerina-extension/src/rpc-managers/visualizer/rpc-manager.ts (1)
120-142: Undo: InlineDataMapper guard correctly prevents unwanted navigationThe added
StateMachine.context().view !== MACHINE_VIEW.InlineDataMapperguard around the PackageOverview navigation aligns with the issue: undo from the inline data mapper will now stay in the inline view even whencurrentArtifactis not found, instead of jumping to the overview. This looks correct for the reported behavior.If you also want consistent behavior when the artifact update times out (lines 136–142), consider applying the same view check there so a slow/no artifact notification does not still kick users from InlineDataMapper to PackageOverview. That would keep the inline experience consistent while still surfacing the timeout error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts(2 hunks)workspaces/ballerina/ballerina-extension/src/rpc-managers/visualizer/rpc-manager.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:136-141
Timestamp: 2025-11-26T07:49:56.428Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBackButton() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
📚 Learning: 2025-11-26T07:49:56.428Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:136-141
Timestamp: 2025-11-26T07:49:56.428Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBackButton() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
Applied to files:
workspaces/ballerina/ballerina-extension/src/rpc-managers/visualizer/rpc-manager.ts
📚 Learning: 2025-11-26T06:34:09.752Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:128-134
Timestamp: 2025-11-26T06:34:09.752Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBreadcrumb() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
Applied to files:
workspaces/ballerina/ballerina-extension/src/rpc-managers/visualizer/rpc-manager.ts
🧬 Code graph analysis (2)
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts (1)
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (1)
undoRedoManager(72-72)
workspaces/ballerina/ballerina-extension/src/rpc-managers/visualizer/rpc-manager.ts (1)
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (1)
StateMachine(793-819)
🔇 Additional comments (1)
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/rpc-manager.ts (1)
54-76: Resetting undo/redo on initial IDM source looks right; confirm global impactImporting
undoRedoManagerand callingundoRedoManager?.reset()right afterupdateSourceingetInitialIDMSourcemakes sense: it effectively treats the freshly generated inline data-mapper source as the new baseline so that undo in the mapper only walks back mapper-triggered edits, which should help avoid the wrong navigation behavior.One thing to double‑check: since
undoRedoManageris a global exported fromstateMachine, this reset will clear any existing undo/redo history managed by the same instance. If other views (non‑inline visualizer flows, other editors) also rely on that shared manager, you might want to confirm that resetting here is acceptable for them, or consider scoping the reset to the relevant file/context in a follow‑up.
Purpose
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.