-
Notifications
You must be signed in to change notification settings - Fork 67
[BI]Remove xml importing functionality #903
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
WalkthroughThe GenericImportTab component removes XML support, converting it to a JSON-only type editor. XML detection logic, import pathways, and related UI elements are deleted, with all references updated to reflect JSON-only functionality. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 (2)
workspaces/ballerina/type-editor/src/TypeEditor/ContextBasedTypeEditor/GenericImportTab.tsx (2)
163-184: Minor redundancy in error handling.The
detectFormatfunction sets an error message at line 179 ("Invalid JSON format"), but this gets immediately overwritten by theuseEffectat line 158 whenUNKNOWNformat is detected. Consider removing line 179'ssetErrorcall since the error will be set by the effect anyway.Apply this diff to remove the redundant error setting:
} catch (e) { // Not valid JSON, continue checking - setError("Invalid JSON format"); }
313-317: Consider simplifying since only JSON is supported.Now that XML support is removed,
handleImportonly needs to handle JSON. The conditional check is somewhat redundant sinceisImportDisabled()already prevents calls with non-JSON formats.Apply this diff to simplify:
const handleImport = async () => { - if (detectedFormat === DetectedFormat.JSON) { - await importAsJson(); - } + await importAsJson(); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workspaces/ballerina/type-editor/src/TypeEditor/ContextBasedTypeEditor/GenericImportTab.tsx(4 hunks)
🔇 Additional comments (3)
workspaces/ballerina/type-editor/src/TypeEditor/ContextBasedTypeEditor/GenericImportTab.tsx (3)
97-101: LGTM! DetectedFormat enum correctly simplified to JSON-only.The removal of XML from the enum is clean and consistent with the PR objectives.
419-420: LGTM! UI text and file restrictions properly updated for JSON-only support.The info banner, file input accept attribute, and placeholder text are all correctly updated to reflect JSON-only functionality.
Also applies to: 429-429, 492-492
97-101: No external dependencies on removed XML functionality found.Verification confirms that the XML support removal from
GenericImportTabis cleanly isolated:
DetectedFormatenum is used only internally within the component; it is not imported or referenced elsewhereGenericImportTabis imported only inContextTypeEditor.tsxand contains no XML-related expectations- XML functionality exists in separate, dedicated components (
RecordFromXml,CreateRecord) and is unaffected by these changes- No external code depends on the removed XML handling
The change introduces no breaking dependencies.
|
@samithkavishke , we may need to keep the XML support for FTP Services isn't it?. To support the |
Yes, we need to support that, but currently XML support is not correctly provided by the language server. |
Purpose
Fixes : wso2/product-ballerina-integrator#1843
Goals
Approach
UI Component Development
npm run storybookfrom the root directory to view current components.Manage Icons
User stories
Issue-1843.mov
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit