[_]: feat/enhance email address validation and availability checks#77
[_]: feat/enhance email address validation and availability checks#77jzunigax2 wants to merge 1 commit into
Conversation
jzunigax2
commented
Jul 18, 2026
jzunigax2
commented
- Updated email address validation logic to include availability checks against the backend.
- Introduced new rules for email address format validation and improved user feedback for taken addresses.
- Enhanced the UI to reflect the loading state during availability checks and updated button states accordingly.
- Added localization support for new validation messages in multiple languages.
- Refactored related tests to cover new functionality and ensure robust validation behavior.
- Updated email address validation logic to include availability checks against the backend. - Introduced new rules for email address format validation and improved user feedback for taken addresses. - Enhanced the UI to reflect the loading state during availability checks and updated button states accordingly. - Added localization support for new validation messages in multiple languages. - Refactored related tests to cover new functionality and ensure robust validation behavior.
|
📝 WalkthroughWalkthroughChangesEmail setup now validates username format and backend availability, handles unavailable or uncertain addresses during submission, supports suggestion-specific labels, and adds localized messages and tests across the service, validation hook, UI, and rule logic. Email availability flow
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 6
🤖 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/hooks/emailAddressRules.test.ts`:
- Line 242: Rewrite the changed test descriptions using plain-language “When …,
then …” behavior statements, without naming implementation symbols. In
src/features/identity-setup/hooks/emailAddressRules.test.ts lines 242-242,
replace the isEmailAddressFormatValid suite description; in
src/features/identity-setup/hooks/useEmailAddressValidation.test.ts lines
30-188, remove hook and callback names from the affected suite and test
descriptions while preserving their behavioral meaning.
- Around line 243-252: Update the affected tests around
isEmailAddressFormatValid to follow the AAA pattern: assign each input during
Arrange, call isEmailAddressFormatValid in a separate Act statement, then assert
the stored result. Add blank lines between the Arrange, Act, and Assert sections
while preserving the existing inputs and expected outcomes.
In `@src/features/identity-setup/hooks/useEmailAddressValidation.test.ts`:
- Line 30: Update the test descriptions within the useEmailAddressValidation
suite to use “When …, then …” wording focused on user-observable situations and
outcomes. Remove references to implementation names such as the hook,
validateAddress, and checkAvailability while preserving each test’s existing
behavior and assertions.
- Around line 161-175: Add a deferred-request test for checkAvailability that
starts with one username/domain, changes the current input before the
availability promise resolves, then resolves the old request successfully and
asserts its result cannot set canSubmit to true or authorize submission. Cover
the stale-result path in useEmailAddressValidation and include the rejection
behavior if the existing test utilities support it.
- Around line 31-39: Update the test setup around beforeEach and afterEach to
call vi.restoreAllMocks() at the start of beforeEach before configuring
checkAddressAvailability.mockResolvedValue. Remove the vi.clearAllMocks() call
from afterEach while preserving fake-timer setup and restoration.
In `@src/features/identity-setup/hooks/useEmailAddressValidation.ts`:
- Around line 56-65: Update checkAvailability in useEmailAddressValidation so
requests are invalidated immediately when the username or domain changes, and
ensure a request whose requestId no longer matches latestRequestIdRef.current
returns a non-success availability state instead of its stale result. Keep
lastCheck updates restricted to the current request, and preserve the existing
valid-result behavior when the request remains current.
🪄 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: 8e165380-9d77-4953-9e6e-e1000e9d99dd
📒 Files selected for processing (13)
src/features/identity-setup/components/EmailAddressRulesPanel.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/hooks/useUpdateEmail.tssrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/it.jsonsrc/services/sdk/mail/index.tssrc/services/sdk/mail/mail.service.test.ts
| }); | ||
| }); | ||
|
|
||
| describe('isEmailAddressFormatValid', () => { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Rewrite implementation-focused test descriptions as plain-language behavior.
src/features/identity-setup/hooks/emailAddressRules.test.ts#L242-L242: replace the function-named suite with a “When …, then …” behavior description.src/features/identity-setup/hooks/useEmailAddressValidation.test.ts#L30-L188: remove references to the hook and callback names from changed suite/test descriptions.
As per coding guidelines, descriptions must use the “When …, then …” pattern without referencing variable, function, or type names.
📍 Affects 2 files
src/features/identity-setup/hooks/emailAddressRules.test.ts#L242-L242(this comment)src/features/identity-setup/hooks/useEmailAddressValidation.test.ts#L30-L188
🤖 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/emailAddressRules.test.ts` at line 242,
Rewrite the changed test descriptions using plain-language “When …, then …”
behavior statements, without naming implementation symbols. In
src/features/identity-setup/hooks/emailAddressRules.test.ts lines 242-242,
replace the isEmailAddressFormatValid suite description; in
src/features/identity-setup/hooks/useEmailAddressValidation.test.ts lines
30-188, remove hook and callback names from the affected suite and test
descriptions while preserving their behavioral meaning.
Source: Coding guidelines
| test('When username satisfies every format rule, then it is valid', () => { | ||
| expect(isEmailAddressFormatValid('jane.doe-99_x')).toBe(true); | ||
| }); | ||
|
|
||
| test('When username is a reserved name, then it is invalid', () => { | ||
| expect(isEmailAddressFormatValid('admin')).toBe(false); | ||
| }); | ||
|
|
||
| test('When username breaks a format rule, then it is invalid', () => { | ||
| expect(isEmailAddressFormatValid('.jane')).toBe(false); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Separate Arrange, Act, and Assert.
These tests invoke the subject inside expect, so the Act and Assert phases are not separated.
Proposed structure
test('When the address satisfies every format rule, then it is valid', () => {
- expect(isEmailAddressFormatValid('jane.doe-99_x')).toBe(true);
+ const result = isEmailAddressFormatValid('jane.doe-99_x');
+
+ expect(result).toBe(true);
});As per coding guidelines, “Structure tests using the AAA pattern (Arrange, Act, Assert) with blank-line separation between sections.”
📝 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.
| test('When username satisfies every format rule, then it is valid', () => { | |
| expect(isEmailAddressFormatValid('jane.doe-99_x')).toBe(true); | |
| }); | |
| test('When username is a reserved name, then it is invalid', () => { | |
| expect(isEmailAddressFormatValid('admin')).toBe(false); | |
| }); | |
| test('When username breaks a format rule, then it is invalid', () => { | |
| expect(isEmailAddressFormatValid('.jane')).toBe(false); | |
| test('When username satisfies every format rule, then it is valid', () => { | |
| const result = isEmailAddressFormatValid('jane.doe-99_x'); | |
| expect(result).toBe(true); | |
| }); | |
| test('When username is a reserved name, then it is invalid', () => { | |
| expect(isEmailAddressFormatValid('admin')).toBe(false); | |
| }); | |
| test('When username breaks a format rule, then it is invalid', () => { | |
| expect(isEmailAddressFormatValid('.jane')).toBe(false); |
🤖 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/emailAddressRules.test.ts` around lines 243
- 252, Update the affected tests around isEmailAddressFormatValid to follow the
AAA pattern: assign each input during Arrange, call isEmailAddressFormatValid in
a separate Act statement, then assert the stored result. Add blank lines between
the Arrange, Act, and Assert sections while preserving the existing inputs and
expected outcomes.
Source: Coding guidelines
| }); | ||
| }; | ||
|
|
||
| describe('useEmailAddressValidation', () => { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove implementation names from behavior descriptions.
Descriptions such as “the hook,” validateAddress, and checkAvailability should describe user-observable situations and outcomes instead.
As per coding guidelines, tests must use “When …, then …” descriptions without referencing variable, function, or type names.
Also applies to: 41-81, 161-188
🤖 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.test.ts` at line
30, Update the test descriptions within the useEmailAddressValidation suite to
use “When …, then …” wording focused on user-observable situations and outcomes.
Remove references to implementation names such as the hook, validateAddress, and
checkAvailability while preserving each test’s existing behavior and assertions.
Source: Coding guidelines
| beforeEach(() => { | ||
| vi.useFakeTimers(); | ||
| checkAddressAvailability.mockResolvedValue({ available: true, suggestion: null }); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.useRealTimers(); | ||
| vi.clearAllMocks(); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Restore mocks in beforeEach.
Use vi.restoreAllMocks() before configuring the default mock, rather than clearing mocks in afterEach.
Proposed fix
beforeEach(() => {
+ vi.restoreAllMocks();
vi.useFakeTimers();
checkAddressAvailability.mockResolvedValue({ available: true, suggestion: null });
});
afterEach(() => {
vi.useRealTimers();
- vi.clearAllMocks();
});As per coding guidelines, “Reset mocks in beforeEach with vi.restoreAllMocks().”
📝 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.
| beforeEach(() => { | |
| vi.useFakeTimers(); | |
| checkAddressAvailability.mockResolvedValue({ available: true, suggestion: null }); | |
| }); | |
| afterEach(() => { | |
| vi.useRealTimers(); | |
| vi.clearAllMocks(); | |
| }); | |
| beforeEach(() => { | |
| vi.restoreAllMocks(); | |
| vi.useFakeTimers(); | |
| checkAddressAvailability.mockResolvedValue({ available: true, suggestion: null }); | |
| }); | |
| afterEach(() => { | |
| vi.useRealTimers(); | |
| }); |
🤖 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.test.ts` around
lines 31 - 39, Update the test setup around beforeEach and afterEach to call
vi.restoreAllMocks() at the start of beforeEach before configuring
checkAddressAvailability.mockResolvedValue. Remove the vi.clearAllMocks() call
from afterEach while preserving fake-timer setup and restoration.
Source: Coding guidelines
| test('When checkAvailability is called on submit, then it re-queries the backend with the current username', async () => { | ||
| const { result } = renderValidationHook(); | ||
| await typeAndSettle(result, 'jane.doe'); | ||
| checkAddressAvailability.mockClear(); | ||
| checkAddressAvailability.mockResolvedValue({ available: false, suggestion: 'jane.doe1@inxt.me' }); | ||
|
|
||
| let outcome; | ||
| await act(async () => { | ||
| outcome = await result.current.checkAvailability(); | ||
| }); | ||
|
|
||
| expect(checkAddressAvailability).toHaveBeenCalledWith('jane.doe', DOMAIN); | ||
| expect(outcome).toEqual({ status: 'taken', suggestion: 'jane.doe1@inxt.me' }); | ||
| expect(result.current.canSubmit).toBe(false); | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Cover a check that becomes stale before resolving.
Add a deferred-request test that changes the username or domain before the manual availability request resolves and asserts that the old successful result cannot authorize submission. This regression test is required for the stale-result path at useEmailAddressValidation.ts Lines 56-65.
As per coding guidelines, tests must cover edge cases and rejection paths.
🤖 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.test.ts` around
lines 161 - 175, Add a deferred-request test for checkAvailability that starts
with one username/domain, changes the current input before the availability
promise resolves, then resolves the old request successfully and asserts its
result cannot set canSubmit to true or authorize submission. Cover the
stale-result path in useEmailAddressValidation and include the rejection
behavior if the existing test utilities support it.
Source: Coding guidelines
| const checkAvailability = useCallback(async (): Promise<AddressAvailability> => { | ||
| if (!isEmailAddressFormatValid(username)) return UNKNOWN_AVAILABILITY; | ||
|
|
||
| const requestId = ++latestRequestIdRef.current; | ||
| const result = await fetchAvailability(username, domain); | ||
| if (requestId === latestRequestIdRef.current) { | ||
| setLastCheck({ value: username, domain, result }); | ||
| } | ||
|
|
||
| return result; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not return a stale availability result to the submit flow.
The request-ID check only protects lastCheck; the stale result is still returned. If the username or domain changes while this request is pending, useUpdateEmail.submit() can accept the old result and advance with values captured before the await. Invalidate requests immediately on input/domain changes and return a non-success state when the request is no longer current.
🤖 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
56 - 65, Update checkAvailability in useEmailAddressValidation so requests are
invalidated immediately when the username or domain changes, and ensure a
request whose requestId no longer matches latestRequestIdRef.current returns a
non-success availability state instead of its stale result. Keep lastCheck
updates restricted to the current request, and preserve the existing
valid-result behavior when the request remains current.
1 participant
