refactor(platform): real-time form validation and legacy integration cleanup#1409
Merged
Israeltheminer merged 3 commits intoApr 11, 2026
Merged
Conversation
… integration dialogs Add isValid prop to FormDialog and wire up onChange validation across all settings forms to disable submit buttons when validation fails. Remove legacy per-integration dialog components (circuly, protel, shopify) now replaced by the generic integration-manage flow. Also fix integration credential labels to use startCase formatting, make branding appName required, and update integrations tab link.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Use exact: false in getByLabelText queries since the required prop adds an aria-label asterisk to the label text.
Contributor
📝 WalkthroughWalkthroughThis PR extends the FormDialog component with form validation state support by introducing an optional Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@services/platform/app/features/settings/branding/components/branding-form.tsx`:
- Line 203: The form currently passes raw validator text to the UI via
errorMessage={formState.errors.appName?.message}; instead, map validation types
to translation keys and pass a localized message (e.g. use the i18n hook t) —
read formState.errors.appName?.type (or code from your resolver) in the
BrandingForm component and convert it to t('settings.branding.errors.required')
/ t('settings.branding.errors.tooLong') etc., then supply that localized string
to the errorMessage prop; update any helper functions in branding-form.tsx that
build validation output to return a translation key or resolved t(...) string
rather than the raw resolver message.
In
`@services/platform/app/features/settings/integrations/components/integration-manage/integration-credentials-form.tsx`:
- Line 234: The label currently uses startCase(binding) directly (see
label={startCase(binding)} in IntegrationCredentialsForm) which bypasses i18n;
change it to resolve a translation key first (e.g., call the component's
translation hook/function to look up something like
"integration.fields.{binding}" with a defaultValue/fallback of
startCase(binding)) and replace both occurrences (the label prop at
startCase(binding) and the similar occurrence around line 253) so the UI shows
localized text while falling back to startCase for missing keys.
In
`@services/platform/app/features/settings/organization/components/member-edit-dialog.tsx`:
- Around line 146-147: The submit button should not be disabled solely because
formState.isValid is false; update the submit gating in MemberEditDialog
(member-edit-dialog.tsx) to allow submission while still preventing duplicate
requests via isSubmitting, and ensure all validation errors are surfaced inline:
render field-level error messages for displayName and other fields, add
aria-invalid on invalid inputs and role="alert" on error message elements, and
remove or relax the isValid check from the submit disabled condition so users
can see and correct errors instead of hitting a dead-end.
In `@services/platform/lib/shared/schemas/branding.ts`:
- Line 24: brandingJsonSchema currently allows appName to be omitted while the
form schema makes appName required; update the persisted schema
(brandingJsonSchema) to require appName (match z.string().min(1).max(100)
semantics used in the form) so there is no schema drift between the form and
persisted data, and add a small backfill/migration to normalize legacy records
where appName is undefined or empty string (set to a sane default or
reject/notify) before enabling the stricter validation; reference the
brandingJsonSchema and the appName field when making these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d267c25c-ef4e-4ba1-8f22-9173c73b3b35
📒 Files selected for processing (20)
services/platform/app/components/ui/dialog/form-dialog.tsxservices/platform/app/features/conversations/components/activate-conversations-empty-state.tsxservices/platform/app/features/settings/account/components/account-form.tsxservices/platform/app/features/settings/api-keys/components/api-key-create-dialog.tsxservices/platform/app/features/settings/branding/components/branding-form.tsxservices/platform/app/features/settings/integrations/components/circuly-disconnect-confirmation-dialog.tsxservices/platform/app/features/settings/integrations/components/circuly-integration-dialog.tsxservices/platform/app/features/settings/integrations/components/integration-manage/integration-credentials-form.tsxservices/platform/app/features/settings/integrations/components/protel-disconnect-confirmation-dialog.tsxservices/platform/app/features/settings/integrations/components/protel-integration-dialog.tsxservices/platform/app/features/settings/integrations/components/shopify-disconnect-confirmation-dialog.tsxservices/platform/app/features/settings/integrations/components/shopify-integration-dialog.tsxservices/platform/app/features/settings/organization/components/member-add-dialog.tsxservices/platform/app/features/settings/organization/components/member-edit-dialog.tsxservices/platform/app/features/settings/providers/components/provider-add-dialog.tsxservices/platform/app/features/settings/teams/components/team-create-dialog.tsxservices/platform/app/features/settings/teams/components/team-edit-dialog.tsxservices/platform/lib/shared/schemas/branding.tsservices/platform/messages/de.jsonservices/platform/messages/en.json
💤 Files with no reviewable changes (6)
- services/platform/app/features/settings/integrations/components/shopify-disconnect-confirmation-dialog.tsx
- services/platform/app/features/settings/integrations/components/protel-disconnect-confirmation-dialog.tsx
- services/platform/app/features/settings/integrations/components/shopify-integration-dialog.tsx
- services/platform/app/features/settings/integrations/components/protel-integration-dialog.tsx
- services/platform/app/features/settings/integrations/components/circuly-disconnect-confirmation-dialog.tsx
- services/platform/app/features/settings/integrations/components/circuly-integration-dialog.tsx
| label={t('branding.appName')} | ||
| placeholder={t('branding.appNamePlaceholder')} | ||
| required | ||
| errorMessage={formState.errors.appName?.message} |
Contributor
There was a problem hiding this comment.
Avoid rendering raw validator messages directly
Line 203 displays resolver text directly, which can leak non-localized/default English validation text to users. Prefer a translation key-driven message.
Suggested fix
- errorMessage={formState.errors.appName?.message}
+ errorMessage={
+ formState.errors.appName
+ ? t('branding.validation.appNameRequired')
+ : undefined
+ }As per coding guidelines: "Do NOT hardcode text, use the translation hooks/functions instead for user-facing UI".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| errorMessage={formState.errors.appName?.message} | |
| errorMessage={ | |
| formState.errors.appName | |
| ? t('branding.validation.appNameRequired') | |
| : undefined | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/branding/components/branding-form.tsx`
at line 203, The form currently passes raw validator text to the UI via
errorMessage={formState.errors.appName?.message}; instead, map validation types
to translation keys and pass a localized message (e.g. use the i18n hook t) —
read formState.errors.appName?.type (or code from your resolver) in the
BrandingForm component and convert it to t('settings.branding.errors.required')
/ t('settings.branding.errors.tooLong') etc., then supply that localized string
to the errorMessage prop; update any helper functions in branding-form.tsx that
build validation output to return a translation key or resolved t(...) string
rather than the raw resolver message.
| key={binding} | ||
| id={`manage-credential-${binding}`} | ||
| label={binding} | ||
| label={startCase(binding)} |
Contributor
There was a problem hiding this comment.
Localize dynamic field labels instead of rendering raw startCase text.
Using startCase(...) directly for visible form labels bypasses i18n and will surface English-like labels in localized UIs. Resolve labels via translation keys first, with startCase only as a fallback.
Proposed fix
- label={startCase(binding)}
+ label={
+ t(`integrations.dynamicFields.${binding}` as never, {
+ defaultValue: startCase(binding),
+ })
+ }
...
- label={startCase(field.key)}
+ label={
+ t(`integrations.dynamicFields.${field.key}` as never, {
+ defaultValue: startCase(field.key),
+ })
+ }Also applies to: 253-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/integrations/components/integration-manage/integration-credentials-form.tsx`
at line 234, The label currently uses startCase(binding) directly (see
label={startCase(binding)} in IntegrationCredentialsForm) which bypasses i18n;
change it to resolve a translation key first (e.g., call the component's
translation hook/function to look up something like
"integration.fields.{binding}" with a defaultValue/fallback of
startCase(binding)) and replace both occurrences (the label prop at
startCase(binding) and the similar occurrence around line 253) so the UI shows
localized text while falling back to startCase for missing keys.
Replace raw validator message with translated error message per i18n coding standards.
Israeltheminer
deleted the
refactor/form-validation-and-integration-cleanup
branch
This was referenced Apr 11, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
isValidprop toFormDialogand enableonChangevalidation mode across all settings forms (account, branding, API keys, teams, members, providers) to disable submit buttons when validation failsstartCaseformattingappNamerequired with inline validation error?tab=allconfigurationi18n key for integration manage dialog (en + de)Test plan
Summary by CodeRabbit
New Features
Improvements
Changes
Localization