fix(auth): use local state for login error to unblock submit button (#648)#670
Conversation
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 adds a comprehensive test suite for the LogInPage component's error handling behavior and modifies the log-in route to track authentication failures using local state rather than form-level error management. The changes introduce a Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
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/auth/__tests__/login-form-error-clearing.test.tsx`:
- Around line 100-104: The test in login-form-error-clearing.test.tsx is
asserting the old behavior that the submit button remains disabled after a
server-side login failure; update the assertion for the button retrieved via
screen.getByRole('button', { name: 'login.loginButton' }) to expect it to be
enabled (toBeEnabled()) once the inputs are schema-valid, and ensure the test
flow still supplies valid credentials/schema-valid input values before asserting
the button state so it reflects the fixed behavior.
- Line 46: The test imports LogInPage from the route module but that symbol
isn't exported, causing TS2459; either export LogInPage from the route module
(e.g., add an export for the LogInPage component/function) or update the test to
import the actual exported symbol (replace LogInPage with the correct exported
name from the route file). Locate the test importing LogInPage and the route
module that defines the login component (the route's exported
component/function), then make the route export and test import agree.
In `@services/platform/app/routes/_auth/log-in.tsx`:
- Line 7: The file imports useState, useMemo, useCallback from React but calls
useEffect later (causing TS2304); update the React import line to also include
useEffect so that the symbol used in the component is defined (i.e., add
useEffect alongside useState/useMemo/useCallback in the import statement).
- Around line 87-90: The loginError state (loginError, setLoginError) is only
cleared inside handleSubmit, so the stale error persists while the user edits
fields; update the email and password input elements used to collect
LogInFormData to call setLoginError(null) on change (e.g., add onChange handlers
to the email and password inputs) so any edit immediately clears the error;
ensure you reference the same controlled/uncontrolled inputs used by the form
(the input elements that populate LogInFormData) and keep existing
value/onChange bindings intact if they exist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0a2dd4e8-b672-48c1-9433-96eb81ea5882
📒 Files selected for processing (2)
services/platform/app/features/auth/__tests__/login-form-error-clearing.test.tsxservices/platform/app/routes/_auth/log-in.tsx
Replace form.setError/clearErrors with local loginError state so that RHF's isValid stays purely Zod-driven and the submit button is never stuck disabled after a failed login attempt. The error message clears automatically when the user edits any field.
72de14c to
3042aa9
Compare
Replace useState/useEffect loginError state with onChange handlers on
form.register() that call form.clearErrors('password') when either field
is edited. RHF manual errors from setError() persist by design — this is
the minimal fix to clear them on user input without extra state.
Replace form.setError/clearErrors with local loginError state so that isValid remains schema-based only, allowing the submit button to re-enable after a failed login attempt when the user edits either field.
2c377f6 to
231e723
Compare
Summary
form.setError()with a localloginErrorstate variable for displaying credential errorsisValid(and therefore the submit button's disabled state) was gated onformState.errorsbeing empty — setting a field error viasetError()permanently disabled the button even after the user corrected the emailisValidstays driven entirely by Zod schema validation, so correcting the email re-enables the button immediatelyTest plan
Closes #648
Summary by CodeRabbit
Bug Fixes
Tests