Skip to content

(feat)ui: migrate form builder in connection form#27812

Open
harsh-vador wants to merge 14 commits intomainfrom
connection-form-builderv1
Open

(feat)ui: migrate form builder in connection form#27812
harsh-vador wants to merge 14 commits intomainfrom
connection-form-builderv1

Conversation

@harsh-vador
Copy link
Copy Markdown
Contributor

@harsh-vador harsh-vador commented Apr 29, 2026

Describe your changes:

Fixes

I worked on ... because ...

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Summary by Gitar

  • Form builder logic:
    • Refactored FormBuilderV1 to memoize mergedFields and mergedWidgets using useMemo for better performance.
  • Field state management:
    • Improved CoreOneOfField to prevent stale state issues by using useEffect and getClosestMatchingOption to track option selection.
    • Added data sanitization in CoreOneOfField when switching options to ensure form data consistency across schema changes.
  • UI enhancements:
    • Updated getWidgetLabel to automatically apply startCase formatting to camelCase field identifiers while preserving existing display labels.
    • Updated FormBuilderV1Widgets.test.tsx to reflect standardized startCase label casing in the UI.
  • Testing:
    • Added CoreOneOfField.test.tsx to verify correct selection persistence when form data matches multiple schema options.
    • Added unit tests for getWidgetLabel to validate label formatting logic for different naming conventions.

This will update automatically on new commits.

@harsh-vador harsh-vador self-assigned this Apr 29, 2026
@harsh-vador harsh-vador requested a review from a team as a code owner April 29, 2026 10:03
@harsh-vador harsh-vador added UI UI specific issues safe to test Add this label to run secure Github workflows on PRs labels Apr 29, 2026
@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented May 4, 2026

Code Review ✅ Approved 3 resolved / 3 findings

Migrates the connection form to the new builder while addressing stale state issues in CoreOneOfField and adding standardized label formatting. All previously identified issues with component naming, stale state, and label casing have been resolved.

✅ 3 resolved
Quality: Missing displayName on forwardRef component FormBuilderV1

📄 openmetadata-ui/src/main/resources/ui/src/components/common/FormBuilderV1/FormBuilderV1.tsx:151
After wrapping FormBuilderV1 with forwardRef, no displayName is assigned. This causes the component to appear as "Anonymous" or "ForwardRef" in React DevTools and may trigger ESLint's react/display-name rule.

Edge Case: CoreOneOfField selectedOption can go stale on formData changes

📄 openmetadata-ui/src/main/resources/ui/src/components/common/FormBuilderV1/fields/CoreOneOfField.tsx:51-61
In CoreOneOfField, selectedOption is initialized from formData via useState(() => ...) but is never updated when formData changes externally (e.g., when a parent resets the form or loads saved data). The component will keep displaying the initially matched option even if formData now matches a different oneOf branch. Consider adding a useEffect to re-derive selectedOption when formData changes from an external source, or use a controlled approach.

Bug: startCase() corrupts acronyms and pre-formatted labels

📄 openmetadata-ui/src/main/resources/ui/src/components/common/FormBuilderV1/widgets/coreWidgetUtils.ts:29 📄 openmetadata-ui/src/main/resources/ui/src/components/common/FormBuilderV1/fields/CoreBooleanField.tsx:23
The getWidgetLabel utility now applies lodash.startCase() to every widget label. startCase splits on word boundaries and capitalizes each word, which means labels like "API Key" become "Api Key", "OAuth2" becomes "O Auth 2", "SSL/TLS" becomes "Ssl Tls", etc. This affects every widget (CoreInputWidget, CoreSelectWidget, CoreTextAreaWidget, etc.) that calls getWidgetLabel. Connection form schemas in OpenMetadata frequently use acronyms (API, SSL, JWT, AWS, GCS, etc.), so this will visibly degrade label quality.

Similarly, CoreBooleanField at line 23 uses startCase(props.name) as a fallback, which has the same issue for field names with acronyms.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 4, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant