-
Notifications
You must be signed in to change notification settings - Fork 67
fix graphql operator forms to conditionally render type field when the ballerinaType exsists #1184
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
…ere is a ballerina type defined for the types object
WalkthroughThe ActionTypeEditor component's conditional rendering logic was updated to verify the existence of a primary input type's ballerinaType attribute instead of checking a general field.types flag. The Type block now only renders when a valid primary input type is available, with its content sourced from and sanitized by the primary type data. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 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-side-panel/src/components/editors/ActionTypeEditor.tsx (1)
593-600: Fix correctly prevents the crash; consider extracting the repeated call.The conditional check successfully addresses the runtime error by verifying
ballerinaTypeexists before rendering. However,getPrimaryInputType(field.types)is called three times within eight lines (lines 593, 596, 598).🔎 Proposed refactor to eliminate redundant calls
- {getPrimaryInputType(field.types)?.ballerinaType && + {(() => { + const primaryType = getPrimaryInputType(field.types); + const ballerinaType = primaryType?.ballerinaType; + return ballerinaType && ( <S.Type isVisible={focused} - title={getPrimaryInputType(field.types)?.ballerinaType} + title={ballerinaType} > - {sanitizeType(getPrimaryInputType(field.types)?.ballerinaType)} + {sanitizeType(ballerinaType)} </S.Type> - } + ); + })()}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ActionTypeEditor.tsx
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
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.
📚 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/ActionTypeEditor.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-side-panel/src/components/editors/ActionTypeEditor.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-side-panel/src/components/editors/ActionTypeEditor.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-side-panel/src/components/editors/ActionTypeEditor.tsx
📚 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-side-panel/src/components/editors/ActionTypeEditor.tsx
📚 Learning: 2025-12-12T13:11:41.252Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 1096
File: workspaces/ballerina/ballerina-extension/src/views/ai-panel/activate.ts:27-28
Timestamp: 2025-12-12T13:11:41.252Z
Learning: In workspaces/ballerina/ballerina-extension/src/views/ai-panel/activate.ts, when processing projectInfo.children in handleWorkspaceLevelAIPanel, the projectPath field is always available for child projects in the workspace, so additional filtering or type guards are not necessary.
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/ActionTypeEditor.tsx
🧬 Code graph analysis (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ActionTypeEditor.tsx (1)
workspaces/ballerina/ballerina-core/src/utils/form-property-utils.ts (1)
getPrimaryInputType(21-24)
Purpose
The GraphQL operators forms were breaking at runtime due to undefined property access. Certain operator configurations attempted to access the
ballerinaTypefield even when the corresponding property was not present, resulting in errors and a broken form experience.Resolves: wso2/product-ballerina-integrator#2169
Goals
Approach
ballerinaTypefield was accessed without verifying the existence of the related property.typefield only when theballerinaTypeproperty exists.Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.