Skip to content

Conversation

@senithkay
Copy link
Contributor

@senithkay senithkay commented Dec 22, 2025

Purpose

Fixed the issue which caused the expression editor to show up instead of the expression editor where the type is record.

Related to: wso2/product-ballerina-integrator#2155

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed service designer to properly include all record types when configuring services, ensuring consistent handling of both standard and derived record type fields.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 22, 2025

Walkthrough

The useEffect hook in ServiceCreationView.tsx has been refactored to aggregate both choice-derived and non-choice record type fields instead of using exclusive conditional branches. The logic now always computes and concatenates both sources into the final recordTypeFields array, with added console logging for debugging.

Changes

Cohort / File(s) Change Summary
Record Type Field Aggregation
workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceCreationView.tsx
Changed useEffect control flow from either-or conditional branches to unified aggregation: now always computes choice-derived fields and non-choice recordTypeFields, concatenates both into final result. Introduced local formRecordTypeFields variable and added debug console logs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Consider whether aggregating both choice-derived and non-choice fields is the intended behavior, or if this represents a functional change
  • Verify that the concatenation order ([...formRecordTypeFields, ...recordTypeFields]) is correct
  • Confirm whether the added console logs are temporary debugging aids or should remain in production code
  • Review the filtering logic for RECORD\_TYPE to ensure correctness

Poem

🐰 Two paths once split, now they unite,
Choice and default dance in the light,
Aggregated fields, a stronger design,
Where rabbit tracks blend, all paths align! ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ⚠️ Warning The PR description is incomplete and lacks required sections from the template including Goals, Approach, UI Component Development, Manage Icons, User stories, Release note, Documentation, Training, Certification, Marketing, Automation tests, Security checks, Samples, Related PRs, Migrations, Test environment, and Learning. Complete the PR description by filling in all required sections from the template, particularly Goals, Approach (with UI screenshots/GIF if applicable), Release note, Documentation, and security verification checkboxes.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main fix: ensuring the record editor appears for top-level record fields in service creation forms, which aligns with the changeset's modification to the record type field aggregation logic.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@RadCod3 RadCod3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (2)
workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceCreationView.tsx (2)

313-316: Remove redundant state update and debug logging.

  1. Line 313: The console.log statement should be removed before production.
  2. Line 315: The setRecordTypeFields(choiceRecordTypeFields) call is redundant since line 340 immediately overwrites this state with the concatenated array.
🔎 Proposed fix
-            console.log(">>> recordTypeFields of http serviceModel", choiceRecordTypeFields);
-
-            setRecordTypeFields(choiceRecordTypeFields);
             formRecordTypeFields = [...choiceRecordTypeFields]

318-339: Fix variable shadowing and remove debug logging.

  1. Line 318: The local variable recordTypeFields shadows the state variable of the same name, which is confusing and could lead to bugs. Consider renaming to topLevelRecordTypeFields for clarity.
  2. Line 339: The console.log statement should be removed before production.
🔎 Proposed fix
-            const recordTypeFields: RecordTypeField[] = Object.entries(model.properties)
+            const topLevelRecordTypeFields: RecordTypeField[] = Object.entries(model.properties)
                 .filter(([_, property]) =>
                     getPrimaryInputType(property.types)?.typeMembers &&
                     getPrimaryInputType(property.types)?.typeMembers.some(member => member.kind === "RECORD_TYPE")
                 )
                 .map(([key, property]) => ({
                     key,
                     property: {
                         ...property,
                         metadata: {
                             label: property.metadata?.label || key,
                             description: property.metadata?.description || ''
                         },
                         types: property.types,
                         diagnostics: {
                             hasDiagnostics: property.diagnostics && property.diagnostics.length > 0,
                             diagnostics: property.diagnostics
                         }
                     } as Property,
                     recordTypeMembers: getPrimaryInputType(property.types)?.typeMembers.filter(member => member.kind === "RECORD_TYPE")
                 }));
-            console.log(">>> recordTypeFields of serviceModel", recordTypeFields);
-            setRecordTypeFields([...formRecordTypeFields, ...recordTypeFields]);
+            console.log(">>> recordTypeFields of serviceModel", topLevelRecordTypeFields);
+            setRecordTypeFields([...formRecordTypeFields, ...topLevelRecordTypeFields]);

Then remove the console.log statement.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 63f64ee and 8bdcc81.

📒 Files selected for processing (1)
  • workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceCreationView.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/**/*.tsx : Use React 18.2.0 features including concurrent rendering and automatic batching; avoid class components in favor of functional components with hooks
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/**/*.tsx : Use React 18.2.0 features including concurrent rendering and automatic batching; avoid class components in favor of functional components with hooks

Applied to files:

  • workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceCreationView.tsx
📚 Learning: 2025-12-16T13:47:11.133Z
Learnt from: kanushka
Repo: wso2/vscode-extensions PR: 1117
File: workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/FormGenerator/index.tsx:1183-1184
Timestamp: 2025-12-16T13:47:11.133Z
Learning: In workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/FormGenerator/index.tsx, the findMatchedType function intentionally uses module?.split('.').pop() to handle simple cases where the last segment of the module name is used for matching. Wildcard imports should be used for collision cases (when different modules have the same last segment), but this is not currently implemented as the code only handles the simple case for now.

Applied to files:

  • workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceCreationView.tsx
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/Diagram.tsx : Implement input validation for CDModel structure: verify that all required properties (connections, listeners, services) are present and properly typed

Applied to files:

  • workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceCreationView.tsx
🧬 Code graph analysis (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceCreationView.tsx (2)
workspaces/ballerina/ballerina-core/src/interfaces/bi.ts (2)
  • RecordTypeField (204-208)
  • Property (176-194)
workspaces/ballerina/ballerina-core/src/utils/form-property-utils.ts (1)
  • getPrimaryInputType (21-24)
🔇 Additional comments (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/ServiceDesigner/ServiceCreationView.tsx (1)

279-342: The refactored logic correctly aggregates both choice-based and top-level record fields, fixing the original bug.

While the code extracts record types from two separate scopes (nested within choices and at the top level), duplication by property key is theoretically possible if a property name appears in both locations. However, this is structurally unlikely given how properties with choices and top-level record types are typically structured in the domain model. The fix appropriately addresses the stated objective of ensuring record editors appear for top-level record-typed fields.

Copy link
Contributor

@NipunaRanasinghe NipunaRanasinghe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@senithkay could you please attach the related issue in the description?

@kanushka kanushka changed the base branch from main to bi-1.6.x December 23, 2025 11:41
@kanushka kanushka merged commit fa88663 into wso2:bi-1.6.x Dec 23, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants