[BI Data Mapper] Fix condition for processing focus inputs in processTypeFields function #1040
Conversation
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
…64595150 Automatic Sync: Merge changes from release-ballerina to main
Contributor
WalkthroughprocessTypeFields in the data-mapper utils now applies focus-input replacement only when the field is in a sequence context (requires Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (8)📓 Common learnings📚 Learning: 2025-11-26T06:34:09.752ZApplied to files:
📚 Learning: 2025-11-26T07:49:56.428ZApplied to files:
📚 Learning: 2025-11-24T14:51:49.267ZApplied to files:
📚 Learning: 2025-11-26T06:37:07.886ZApplied to files:
📚 Learning: 2025-11-26T06:35:19.217ZApplied to files:
📚 Learning: 2025-11-26T06:33:22.950ZApplied to files:
📚 Learning: 2025-11-20T06:44:07.498ZApplied to files:
🔇 Additional comments (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts (2)
730-747: Seq‑gated focus processing logic looks consistent but is quite subtleTying focus member resolution to
isSeq(derived fromgroupById) plus keepingfocusInputRootMapupdates inside that guard matches the idea that focus inputs should only apply in sequence/group‑by contexts, and avoids incorrectly mirroring focus for non‑seq fields. The extraif (fieldId === model.groupById) { isSeq = false; }is a bit non‑obvious though; consider a short comment explaining why the group‑by field itself should not be treated as seq for future maintainers, since this is easy to regress. Based on learnings, this keeps focusInputRootMap semantics aligned with how focus views are resolved in the diagram layer.
748-757: RemovingisFocused/isSeqfrom IOType changes its contract—confirm no remaining consumersCommenting out the spread of
isFocusedandisSeqmeans these flags no longer appear on emittedIOTypeinstances fromprocessTypeFields. That’s fine if callers (e.g., the data‑mapper UI) have been updated to not rely on them, but it is a silent contract change; please double‑check there are no remaining usages expecting these properties. If they’re confirmed unused, you can follow up by removing the commented lines (and any now‑redundant plumbing) to keep this path lean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 969
File: workspaces/ballerina/data-mapper/src/components/Diagram/utils/node-utils.ts:23-44
Timestamp: 2025-11-20T06:44:07.498Z
Learning: In the data mapper node-utils.ts, when `lastViewIndex == 0`, the code is at the root view and should NOT use `focusInputRootMap` for fallback input node resolution. The truthiness check on `lastViewIndex` is intentional to skip the fallback at the root level.
📚 Learning: 2025-11-20T06:44:07.498Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 969
File: workspaces/ballerina/data-mapper/src/components/Diagram/utils/node-utils.ts:23-44
Timestamp: 2025-11-20T06:44:07.498Z
Learning: In the data mapper node-utils.ts, when `lastViewIndex == 0`, the code is at the root view and should NOT use `focusInputRootMap` for fallback input node resolution. The truthiness check on `lastViewIndex` is intentional to skip the fallback at the root level.
Applied to files:
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.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/data-mapper/utils.ts
📚 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/data-mapper/utils.ts
📚 Learning: 2025-11-24T14:51:49.267Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 998
File: workspaces/ballerina/data-mapper/src/components/DataMapper/Header/ExpressionBar.tsx:113-132
Timestamp: 2025-11-24T14:51:49.267Z
Learning: In workspaces/ballerina/data-mapper/src/components/DataMapper/Header/ExpressionBar.tsx, if `textFieldRef.current` is not undefined, `textFieldRef.current.inputElement` is guaranteed to exist. If `inputElement` doesn't exist when `current` exists, it's a fatal error that should reach the error boundary rather than being handled with defensive null checks.
Applied to files:
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts
kanushka
approved these changes
Dec 2, 2025
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.
Purpose
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.