[_]: feat/email validation when setting address up#73
Conversation
|
Warning Review limit reached
Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughIdentity setup now uses shared, debounced email username validation with per-rule feedback, focus-aware rendering, and validity-gated form submission. Confirmation components also use form submit handlers, while localized validation guidance and error messages are updated. ChangesIdentity setup validation flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/i18n/locales/en.json (1)
160-161: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd the missing English username validation key.
src/i18n/locales/en.jsonis missingerrors.identitySetup.usernameInvalid;es.json,fr.json, andit.jsonalready have it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/i18n/locales/en.json` around lines 160 - 161, Add the missing errors.identitySetup.usernameInvalid translation key to the English locale, matching the existing identitySetup structure and the corresponding validation message used in the other locales.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/identity-setup/components/ConfirmPassword.tsx`:
- Around line 21-25: Replace React.FormEvent with
React.SubmitEvent<HTMLFormElement> in the handleSubmit form handlers for
ConfirmPassword.tsx and ConfirmChange.tsx, preserving their existing submission
behavior.
In `@src/features/identity-setup/components/SelectMailInput.tsx`:
- Around line 15-24: Extract the isFocused state and its related focus event
handling from SelectMailInput into a custom hook such as useSelectMailInput
under the feature’s hooks/ directory or alongside the component. Update
SelectMailInput to consume the hook and remain focused on rendering and prop
wiring.
- Line 46: Update the input element’s Tailwind class in SelectMailInput to use
outline-hidden instead of outline-none, preserving the existing styling and
accessibility behavior.
In `@src/features/identity-setup/components/UpdateEmail.tsx`:
- Around line 18-52: Extract the state, validation, and submission logic from
the UpdateEmail component into a useUpdateEmail custom hook, including domain,
focus state, descriptions, panel visibility, and the submit handler. Remove
handleFormKeyDown and handleConfirm; have the hook’s handleSubmit prevent
default, validate isValid, and call onNext directly. Keep UpdateEmail focused on
rendering the hook-provided values and handlers, with the form using only
onSubmit.
In `@src/features/identity-setup/hooks/emailAddressRules.test.ts`:
- Around line 14-34: Refactor every test in the file, including the length-rule
tests and other test cases, to follow Arrange–Act–Assert structure: set up
inputs and dependencies first, invoke ruleStatus in a separate Act section, then
perform assertions. Add blank lines separating each section while preserving all
existing test behavior and expectations.
In `@src/features/identity-setup/hooks/useEmailAddressValidation.test.ts`:
- Around line 6-8: Update the beforeEach setup in the email address validation
tests to call vi.restoreAllMocks() so mocks are reset before every test, while
preserving the existing fake-timer initialization.
In `@src/features/identity-setup/hooks/useEmailAddressValidation.ts`:
- Around line 25-27: Update the isValid calculation in the email validation hook
to return false whenever username differs from debouncedUsername, while
preserving the existing rules.every validation when both values are
synchronized. Include username in the relevant memo dependencies so rapid input
cannot submit based on stale validation.
---
Outside diff comments:
In `@src/i18n/locales/en.json`:
- Around line 160-161: Add the missing errors.identitySetup.usernameInvalid
translation key to the English locale, matching the existing identitySetup
structure and the corresponding validation message used in the other locales.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 70f50da3-fe96-4647-b699-10daa649f619
📒 Files selected for processing (14)
src/features/identity-setup/components/ConfirmChange.tsxsrc/features/identity-setup/components/ConfirmPassword.tsxsrc/features/identity-setup/components/EmailAddressRulesPanel.tsxsrc/features/identity-setup/components/SelectMailInput.tsxsrc/features/identity-setup/components/UpdateEmail.tsxsrc/features/identity-setup/hooks/emailAddressRules.test.tssrc/features/identity-setup/hooks/emailAddressRules.tssrc/features/identity-setup/hooks/useEmailAddressValidation.test.tssrc/features/identity-setup/hooks/useEmailAddressValidation.tssrc/features/identity-setup/index.tsxsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/it.json
| const handleSubmit = (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
| onConfirmPassword(); | ||
| }; | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Prefer React.SubmitEvent over React.FormEvent for form submissions.
Both files type the form submission event as React.FormEvent. Based on learnings, FormEvent is deprecated in React 19.2+ in favor of SubmitEvent. Please update the event types for form handlers to React.SubmitEvent<HTMLFormElement>.
src/features/identity-setup/components/ConfirmPassword.tsx#L21-L25: changee: React.FormEventtoe: React.SubmitEvent<HTMLFormElement>.src/features/identity-setup/components/ConfirmChange.tsx#L27-L32: changee: React.FormEventtoe: React.SubmitEvent<HTMLFormElement>.
📍 Affects 2 files
src/features/identity-setup/components/ConfirmPassword.tsx#L21-L25(this comment)src/features/identity-setup/components/ConfirmChange.tsx#L27-L32
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/identity-setup/components/ConfirmPassword.tsx` around lines 21 -
25, Replace React.FormEvent with React.SubmitEvent<HTMLFormElement> in the
handleSubmit form handlers for ConfirmPassword.tsx and ConfirmChange.tsx,
preserving their existing submission behavior.
Source: Learnings
| const { username, rules, isValid, validateAddress } = useEmailAddressValidation(); | ||
| const [domain, setDomain] = useState<string>(activeDomains[0]?.domain ?? ''); | ||
| const [usernameError, setUsernameError] = useState<string>(''); | ||
| const [isUsernameFocused, setIsUsernameFocused] = useState(false); | ||
|
|
||
| const descriptions = translateArray('identitySetup.updateEmail.description', { | ||
| name: userFullName, | ||
| current_email: currentEmail, | ||
| }); | ||
|
|
||
| const handleConfirm = () => { | ||
| if (!username.trim()) { | ||
| setUsernameError(translate('errors.identitySetup.usernameRequired')); | ||
| return; | ||
| } | ||
| if (!isValid) return; | ||
| onNext({ address: username, domain }); | ||
| }; | ||
|
|
||
| const handleSubmit = (e: React.SubmitEvent) => { | ||
| e.preventDefault(); | ||
| handleConfirm(); | ||
| }; | ||
|
|
||
| const handleFormKeyDown = (e: React.KeyboardEvent<HTMLFormElement>) => { | ||
| if (e.key !== 'Enter' || (e.target as HTMLElement).tagName === 'BUTTON') return; | ||
|
|
||
| e.preventDefault(); | ||
| handleConfirm(); | ||
| }; | ||
|
|
||
| const hasStartedTyping = rules.some((rule) => rule.status !== 'idle'); | ||
| const isPanelVisible = isUsernameFocused || hasStartedTyping; | ||
|
|
||
| return ( | ||
| <div className="flex flex-col gap-5 justify-center items-center"> | ||
| <form | ||
| onSubmit={handleSubmit} | ||
| onKeyDown={handleFormKeyDown} | ||
| className="flex flex-col gap-5 justify-center items-center" | ||
| > |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Extract logic into a custom hook and remove redundant event handler.
As per path instructions, components must be "dumb" and only render; all state and event handlers should be extracted into a custom hook (e.g., useUpdateEmail).
Additionally, the handleFormKeyDown function is redundant. Since the input is within a <form> containing a <Button type="submit">, pressing the Enter key inside the input will natively trigger the onSubmit event. You can safely remove handleFormKeyDown and handleConfirm, moving the validation and submission logic directly into handleSubmit within your new hook.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/identity-setup/components/UpdateEmail.tsx` around lines 18 - 52,
Extract the state, validation, and submission logic from the UpdateEmail
component into a useUpdateEmail custom hook, including domain, focus state,
descriptions, panel visibility, and the submit handler. Remove handleFormKeyDown
and handleConfirm; have the hook’s handleSubmit prevent default, validate
isValid, and call onNext directly. Keep UpdateEmail focused on rendering the
hook-provided values and handlers, with the form using only onSubmit.
Source: Path instructions
| const rules = useMemo(() => validateEmailAddress(debouncedUsername), [debouncedUsername]); | ||
| const isValid = useMemo(() => rules.every((rule) => isRuleStatusValid(rule.status)), [rules]); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Prevent submission of invalid input during debounce delay.
Currently, isValid evaluates debouncedUsername. If the user types rapidly and triggers the submit action before the debounce delay elapses, isValid may still return true based on the previous valid state. This allows the form to submit an unvalidated, potentially invalid username (TOCTOU flaw).
To prevent this, isValid should evaluate to false whenever username is out of sync with debouncedUsername.
🐛 Proposed fix
const rules = useMemo(() => validateEmailAddress(debouncedUsername), [debouncedUsername]);
- const isValid = useMemo(() => rules.every((rule) => isRuleStatusValid(rule.status)), [rules]);
+ const isSynchronized = username === debouncedUsername;
+ const isValid = useMemo(
+ () => isSynchronized && rules.every((rule) => isRuleStatusValid(rule.status)),
+ [rules, isSynchronized]
+ );📝 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.
| const rules = useMemo(() => validateEmailAddress(debouncedUsername), [debouncedUsername]); | |
| const isValid = useMemo(() => rules.every((rule) => isRuleStatusValid(rule.status)), [rules]); | |
| const rules = useMemo(() => validateEmailAddress(debouncedUsername), [debouncedUsername]); | |
| const isSynchronized = username === debouncedUsername; | |
| const isValid = useMemo( | |
| () => isSynchronized && rules.every((rule) => isRuleStatusValid(rule.status)), | |
| [rules, isSynchronized] | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/identity-setup/hooks/useEmailAddressValidation.ts` around lines
25 - 27, Update the isValid calculation in the email validation hook to return
false whenever username differs from debouncedUsername, while preserving the
existing rules.every validation when both values are synchronized. Include
username in the relevant memo dependencies so rapid input cannot submit based on
stale validation.
Deploying mail-web with
|
| Latest commit: |
c83e6fd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://50f0e63a.mail-web-ea0.pages.dev |
| Branch Preview URL: | https://feature-check-som-rules-of-u.mail-web-ea0.pages.dev |
|
2 participants
Checking the address user before creating it so we can validate it follows some predefined standards, like: