refactor(ui): adopt react-hook-form for remaining auth forms#6606
refactor(ui): adopt react-hook-form for remaining auth forms#6606luannmoreira wants to merge 1 commit into
Conversation
|
/review |
|
Claude finished @otavio's task in 7m 10s —— View job Code Review CompleteReviewed 24 files across code quality, security, testing, language patterns, and architecture. 1 inline issue posted (1 medium). The RHF migration is clean and consistent with the SignUp pilot. Hook ordering is correct ( The one finding: in Additional notes (non-blocking, not posted inline)
If you've addressed the feedback and want a new review, tag |
|
Thanks for the thorough review. Addressed the inline finding plus two of the non-blocking notes in cc82393:
Leaving the third note (resolver placement under |
Trim the recovery code in MfaRecover's Disable-MFA call so it matches the resolver-trimmed value used for recovery; a whitespace-padded code no longer passes recovery but fails the disable escape hatch. Strengthen two tests: assert the Login 403 redirect uses the trimmed username, and drive MfaRecover's error test through the store action's real behavior instead of injecting the error state after submit.
|
/review |
|
Claude finished @otavio's task in 5m 15s —— View job Code Review CompleteReviewed 24 files across code quality, security, testing, language patterns, and architecture. 1 inline issue posted (1 low). The follow-up commit (
Other areas checked out: hook ordering ( The one finding
If you've addressed the feedback and want a new review, tag |
|
@luannmoreira Besides the comments that the code review has raised, I think that you can do the move of the files, as for the git, it's very cheap and are not going to be shown in the DIFF, so it's not going to be hard to review either way. |
|
Done in 6805c63 — moved |
|
/review |
|
Claude finished @otavio's task in 6m 18s —— View job Code Review CompleteReviewed 24 files across code quality, security, testing, language patterns, and architecture. 1 inline issue posted (1 low). The migration is a faithful, security-neutral refactor. I re-verified everything the prior rounds flagged and confirmed the current state is clean:
The one finding
Non-blocking notes (not posted inline)
If you've addressed the feedback and want a new review, tag |
67d2ab3 to
5b3c0f4
Compare
|
/review |
|
Claude finished @otavio's task in 7m 58s —— View job Code Review CompleteReviewed 24 files across code quality, security, testing, language patterns, and architecture. 1 inline issue posted (1 low). This is a faithful, security-neutral RHF migration, and the three items from prior rounds are confirmed fixed in the current source:
Everything else re-verified clean:
The one finding
If you've addressed the feedback and want a new review, tag |
5b3c0f4 to
ebed9f6
Compare
|
/review |
|
Claude finished @otavio's task in 5m 55s —— View job Code Review CompleteReviewed 24 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is. All items from the prior four rounds are confirmed fixed in the current source:
Re-verified clean this round:
If you push additional changes and want a new review, tag |
What
Migrate the remaining auth and onboarding forms in
@shellhub/consoleontothe react-hook-form pattern established by the SignUp pilot (#6597): Login,
ForgotPassword, UpdatePassword, Setup, and MfaRecover move to
useForm+ aresolver adapter over the existing validators. Stacked on #6597; merge after it.
Why
Advances the Auth & onboarding section of the incremental RHF migration tracked
in shellhub-io/team#152. Each form previously hand-threaded
useState+onChange+touched+ memoized validation; this replaces that with the sharedRHF wrappers and per-form resolvers.
Changes
loginResolver,forgotPasswordResolver,updatePasswordResolver,mfaRecoverResolver(beside their pages) andsetupResolver(inpages/setup/, alongsidevalidate.ts/signUpResolver.ts).Each reuses the existing validators rather than duplicating rules.
utils/validation.ts— login/recoveridentifiers accept a username OR an email, trimmed before validation. The
login/forgot resolvers return the trimmed value so submit and the 403 redirect
use it. Login password stays ungated on length so pre-existing credentials
still authenticate.
<input>viaregister(preserving the yellow focus ring / mono font instead of swapping to
FormInputField) and reads the value throughgetValues, so the recoverytimeout modal's Disable-MFA action still sends the real code.
resetFieldreplaces the manual state clear on failure.
useFormwrapper for a uniform submit path; thehook precedes the identifier early-return guard. No field validation (no inputs).
flow, success redirect, and back button are unchanged.
page with no input fields, so RHF has nothing to bind.
pilot's style.
Testing
vitest,tsc --noEmit, andeslintall run inside theuicontainer: 198tests pass across the touched suites. Worth probing: the MfaRecover Disable-MFA
path (must send the actual recovery code, not an empty string) and Login's
username trimming (submit and the 403 confirm-account redirect must use the
trimmed value).