feat(platform): add isInvalid input prop, refactor auth errors, drop custom integrations#962
Conversation
…custom integrations - Add `isInvalid` prop to Input component for visual error state without requiring an error message - Refactor login page to show error as standalone alert instead of attaching to password field - Guard sign-up page: redirect to login if users already exist - Remove "custom" integration tab, badge, and filtering logic — all integrations are now treated uniformly - Always show delete button in integration panel - Make CRM assistant agent visible in chat
![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.
📝 WalkthroughWalkthroughThis pull request removes the "custom" integrations classification across the platform, refactors login form error display to use a separate alert block instead of form field error messages, adds a user existence check to the sign-up page, and updates the CRM assistant agent configuration. Changes span example configurations, UI components (Input, IntegrationCard, IntegrationPanel, Integrations list), route logic (login, sign-up), tests, and localization strings. The 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)
⚔️ Resolve merge conflicts
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/app/features/settings/integrations/components/integrations.tsx (1)
22-33: 🧹 Nitpick | 🔵 TrivialConsider adding
isActivetoIntegrationListIteminterface.The code accesses
integration.isActiveat lines 68 and 149, butisActiveis not explicitly declared in theIntegrationListIteminterface. It currently relies on the catch-all[key: string]: unknownindex signature, which defeats type safety—isActivecould be any type or missing entirely.♻️ Proposed fix to add explicit property
export interface IntegrationListItem { _id: string; slug: string; title: string; description?: string; installed: boolean; + isActive?: boolean; type?: 'rest_api' | 'sql'; authMethod: string; operationCount: number; hash: string; [key: string]: unknown; }Also applies to: 68-68, 149-149
🤖 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/integrations.tsx` around lines 22 - 33, The IntegrationListItem interface is missing an explicit isActive property which is accessed as integration.isActive in the component; add isActive?: boolean to the IntegrationListItem declaration so TypeScript enforces its type, then update call sites that read integration.isActive (e.g., the checks in the integrations component where integration.isActive is used) to treat it as a boolean (coalesce to false if undefined) or ensure providers supply it, ensuring type safety across the component.
🤖 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/routes/_auth/log-in.tsx`:
- Around line 173-181: The error block rendering loginError should include the
same icon used by the Input error messages for visual consistency: import and
render the same icon (e.g., XCircle or Info) before the {loginError} text inside
the paragraph that currently uses className="text-destructive flex items-center
gap-1.5 text-sm"; update the JSX for the loginError block in the
_auth/log-in.tsx route to place the icon component (matching the Input
component's usage) as the first child while preserving role="alert" and
aria-live="polite" so styling and accessibility remain consistent with the Input
component.
In `@services/platform/app/routes/_auth/sign-up.tsx`:
- Around line 130-132: The current check in the SignUp route that returns null
when isLoadingUsers or hasUsers === true should instead render an accessible
status message; replace the blank return with an element using an aria-live
region (e.g., <div role="status" aria-live="polite">) that displays a translated
message so screen readers receive feedback. Use the existing translation
function (e.g., t or i18n) with a key like "signUp.loading" or
"signUp.redirecting" and render that string when isLoadingUsers or hasUsers is
true; ensure the element is lightweight and visually unobtrusive but present in
the DOM instead of returning null.
---
Outside diff comments:
In
`@services/platform/app/features/settings/integrations/components/integrations.tsx`:
- Around line 22-33: The IntegrationListItem interface is missing an explicit
isActive property which is accessed as integration.isActive in the component;
add isActive?: boolean to the IntegrationListItem declaration so TypeScript
enforces its type, then update call sites that read integration.isActive (e.g.,
the checks in the integrations component where integration.isActive is used) to
treat it as a boolean (coalesce to false if undefined) or ensure providers
supply it, ensuring type safety across the component.
🪄 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: b2155095-d287-44c6-ac56-07afd7bd1200
📒 Files selected for processing (12)
examples/agents/crm-assistant.jsonservices/platform/app/components/ui/forms/input.tsxservices/platform/app/features/auth/__tests__/login-form-error-clearing.test.tsxservices/platform/app/features/settings/integrations/components/__tests__/integration-card.test.tsxservices/platform/app/features/settings/integrations/components/__tests__/integrations.test.tsxservices/platform/app/features/settings/integrations/components/integration-card.tsxservices/platform/app/features/settings/integrations/components/integration-panel.tsxservices/platform/app/features/settings/integrations/components/integration-upload/integration-upload-dialog.tsxservices/platform/app/features/settings/integrations/components/integrations.tsxservices/platform/app/routes/_auth/log-in.tsxservices/platform/app/routes/_auth/sign-up.tsxservices/platform/messages/en.json
💤 Files with no reviewable changes (1)
- services/platform/app/features/settings/integrations/components/tests/integrations.test.tsx
| {loginError && ( | ||
| <p | ||
| role="alert" | ||
| aria-live="polite" | ||
| className="text-destructive flex items-center gap-1.5 text-sm" | ||
| > | ||
| {loginError} | ||
| </p> | ||
| )} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding an icon to match the Input component's error styling.
The standalone error block uses the same classes as the Input component's error message (text-destructive flex items-center gap-1.5 text-sm) but omits the icon. The Input component renders <XCircle> or <Info> before the error text. Adding an icon here would maintain visual consistency.
🎨 Optional: Add icon for visual consistency
+import { XCircle } from 'lucide-react';
+
// ... in JSX:
{loginError && (
<p
role="alert"
aria-live="polite"
className="text-destructive flex items-center gap-1.5 text-sm"
>
+ <XCircle className="size-4" aria-hidden="true" />
{loginError}
</p>
)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/routes/_auth/log-in.tsx` around lines 173 - 181, The
error block rendering loginError should include the same icon used by the Input
error messages for visual consistency: import and render the same icon (e.g.,
XCircle or Info) before the {loginError} text inside the paragraph that
currently uses className="text-destructive flex items-center gap-1.5 text-sm";
update the JSX for the loginError block in the _auth/log-in.tsx route to place
the icon component (matching the Input component's usage) as the first child
while preserving role="alert" and aria-live="polite" so styling and
accessibility remain consistent with the Input component.
| if (isLoadingUsers || hasUsers === true) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Avoid null for loading/redirect state; provide accessible status feedback.
Line 130-132 currently renders a blank screen, which gives no visual or screen-reader feedback during loading/redirect. Render a translated status message in an aria-live region instead.
Suggested patch
- if (isLoadingUsers || hasUsers === true) {
- return null;
- }
+ if (isLoadingUsers) {
+ return (
+ <p aria-live="polite" className="text-sm text-muted-foreground">
+ {t('signup.checkingAccess')}
+ </p>
+ );
+ }
+
+ if (hasUsers === true) {
+ return (
+ <p aria-live="polite" className="text-sm text-muted-foreground">
+ {t('signup.redirectingToLogin')}
+ </p>
+ );
+ }As per coding guidelines, "Use aria-live regions for dynamic content updates."
📝 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.
| if (isLoadingUsers || hasUsers === true) { | |
| return null; | |
| } | |
| if (isLoadingUsers) { | |
| return ( | |
| <p aria-live="polite" className="text-sm text-muted-foreground"> | |
| {t('signup.checkingAccess')} | |
| </p> | |
| ); | |
| } | |
| if (hasUsers === true) { | |
| return ( | |
| <p aria-live="polite" className="text-sm text-muted-foreground"> | |
| {t('signup.redirectingToLogin')} | |
| </p> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/routes/_auth/sign-up.tsx` around lines 130 - 132, The
current check in the SignUp route that returns null when isLoadingUsers or
hasUsers === true should instead render an accessible status message; replace
the blank return with an element using an aria-live region (e.g., <div
role="status" aria-live="polite">) that displays a translated message so screen
readers receive feedback. Use the existing translation function (e.g., t or
i18n) with a key like "signUp.loading" or "signUp.redirecting" and render that
string when isLoadingUsers or hasUsers is true; ensure the element is
lightweight and visually unobtrusive but present in the DOM instead of returning
null.
Merge main's supportedModels field with our description and visibleInChat changes.
Israeltheminer
deleted the
feat/auth-input-validation-and-integrations-cleanup
branch
…custom integrations (#962)
1 participant
Summary
isInvalidprop to show visual error state (border + shake) without requiring an error message stringTest plan
source: 'custom'metadataSummary by CodeRabbit
Release Notes
New Features
Improvements
Refactor