fix function form saving without entered parameters #1183
Merged
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
Contributor
WalkthroughThe changes refactor handling of repeatable properties with template types: EditorFactory conditionally routes template-based repeatable properties to ParamManagerEditor; the visualizer stops auto-assigning PARAM_MANAGER; FunctionForm uses the template constraint for per-iteration values instead of ballerinaType. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (7)📚 Learning: 2025-11-24T22:16:28.380ZApplied to files:
📚 Learning: 2025-11-25T06:34:10.812ZApplied to files:
📚 Learning: 2025-11-25T06:34:10.812ZApplied to files:
📚 Learning: 2025-12-16T13:47:11.133ZApplied to files:
📚 Learning: 2025-11-25T06:34:10.812ZApplied to files:
📚 Learning: 2025-11-25T06:34:10.812ZApplied to files:
📚 Learning: 2025-11-25T06:34:10.812ZApplied to files:
🧬 Code graph analysis (1)workspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.tsx (1)
🔇 Additional comments (2)
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 (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionForm/index.tsx (1)
246-259: Core fix correctly aligns with the refactored property model.This change properly addresses the bug by using
primaryType.templatefrom the new types array instead of the legacyballerinaTypeproperty. The deep clone viaJSON.parse(JSON.stringify(template))is appropriate for this template structure.Minor:
repeatKeyon line 251 is declared but unused. Consider using an underscore prefix to indicate intentional discard:🔎 Optional: Mark unused variable
- for (const [repeatKey, repeatValue] of Object.entries(dataValue)) { + for (const [_repeatKey, repeatValue] of Object.entries(dataValue)) {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
workspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.tsxworkspaces/ballerina/ballerina-visualizer/src/utils/bi.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionForm/index.tsx
💤 Files with no reviewable changes (1)
- workspaces/ballerina/ballerina-visualizer/src/utils/bi.tsx
🧰 Additional context used
🧠 Learnings (9)
📚 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/FunctionForm/index.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.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/**/*.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/FunctionForm/index.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.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/nodes/EntryNode/components/**/*.tsx : Implement lazy expansion of functions: only render function items when they are visible to improve performance for large function lists
Applied to files:
workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionForm/index.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/**/*.{ts,tsx} : Define all constants (node types, sizing, spacing) in src/resources/constants.ts and import them where needed instead of hardcoding values
Applied to files:
workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionForm/index.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.tsx
📚 Learning: 2025-11-27T07:59:33.534Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 897
File: workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClausesPanel.tsx:50-59
Timestamp: 2025-11-27T07:59:33.534Z
Learning: In workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClausesPanel.tsx, the `clause.properties.expression` property in the `fillDefaults` function does not require defensive null/undefined checks because it's a required property enforced by form validation in ClauseEditor.
Applied to files:
workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionForm/index.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/**/*.{ts,tsx} : Use TypeScript 5.8.3 with strict type checking enabled for all source files
Applied to files:
workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionForm/index.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.tsx
📚 Learning: 2025-11-24T22:16:28.380Z
Learnt from: dan-niles
Repo: wso2/vscode-extensions PR: 980
File: workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RichTextTemplateEditor/RichTextTemplateEditor.tsx:384-412
Timestamp: 2025-11-24T22:16:28.380Z
Learning: In the RichTextTemplateEditor component (workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RichTextTemplateEditor/RichTextTemplateEditor.tsx), token fetching on external `value` prop changes is intentionally disabled. Users cannot edit both the minimized and expanded editors simultaneously, so tokens only need to be generated based on user typing in the expanded editor view, not on external prop updates.
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.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/nodes/**/!(*Widget|*Model).tsx : Implement the factory pattern for node instantiation: each node type (Listener, Entry, Connection) must have a corresponding Factory class that extends the appropriate factory interface
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.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/nodes/**/*.{ts,tsx} : Each node type must have a Model class extending NodeModel, a Factory class implementing the factory pattern, and a Widget React component for visual representation
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.tsx
🧬 Code graph analysis (2)
workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionForm/index.tsx (1)
workspaces/ballerina/ballerina-core/src/utils/form-property-utils.ts (2)
getPrimaryInputType(21-24)isTemplateType(26-30)
workspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.tsx (1)
workspaces/ballerina/ballerina-core/src/utils/form-property-utils.ts (2)
isTemplateType(26-30)getPrimaryInputType(21-24)
🔇 Additional comments (2)
workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionForm/index.tsx (1)
117-121: LGTM: Properly handles template-based parameter description.The logic correctly identifies template types and sets the nested
parameterDescriptiontype to"TEXTAREA". Theas anycasts are acceptable here given the dynamic nature of the template structure.workspaces/ballerina/ballerina-side-panel/src/components/editors/EditorFactory.tsx (1)
206-209: Good fix aligning with the refactored property model.The conditional routing based on
isTemplateTypecorrectly distinguishes between template-based repeatable properties (usingParamManagerEditor) and others (usingFormMapEditor). This properly replaces the removedPARAM_MANAGERtype-specific branch.Note:
getPrimaryInputTypereturnsInputType | undefined, and whileisTemplateTypehandles this safely at runtime (thetypeof value === "object"check returnsfalseforundefined), you may want to add an explicit null check for defensive robustness, depending on your code standards.
sachiniSam
reviewed
Dec 23, 2025
| // Skip this property | ||
| return <></>; | ||
| } else if (field.type === "PARAM_MANAGER") { | ||
| } else if (field.type === "REPEATABLE_PROPERTY" && isTemplateType(getPrimaryInputType(field.types))) { |
Contributor
There was a problem hiding this comment.
if we are removing PARAM_MANAGER from the editor factory we need to refactor all the forms which right now uses the PARAM_MANAGER and verify if those apis as well support this new REPEATABLE_PROPERTY type to generate source from the model.
sachiniSam
approved these changes
Dec 23, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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
The Create Function form did not persist function parameters when saving. This issue was caused by parts of the save logic still relying on the legacy property model that existed prior to the recent refactoring, leading to a mismatch between the expected and provided request objects.
Resolves issue: wso2/product-ballerina-integrator#2117
Goals
Approach
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.