-
Notifications
You must be signed in to change notification settings - Fork 1
[_]: feat/enhance email address validation and availability checks #77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,14 +1,19 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| import { describe, expect, test } from 'vitest'; | ||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||
| isEmailAddressFormatValid, | ||||||||||||||||||||||||||||||||||||||||||||||
| isEmailAddressValid, | ||||||||||||||||||||||||||||||||||||||||||||||
| isRuleStatusValid, | ||||||||||||||||||||||||||||||||||||||||||||||
| validateEmailAddress, | ||||||||||||||||||||||||||||||||||||||||||||||
| type AddressAvailability, | ||||||||||||||||||||||||||||||||||||||||||||||
| type EmailAddressRuleId, | ||||||||||||||||||||||||||||||||||||||||||||||
| type EmailAddressRuleStatus, | ||||||||||||||||||||||||||||||||||||||||||||||
| } from './emailAddressRules'; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const ruleStatus = (username: string, id: EmailAddressRuleId): EmailAddressRuleStatus => | ||||||||||||||||||||||||||||||||||||||||||||||
| validateEmailAddress(username).find((rule) => rule.id === id)!.status; | ||||||||||||||||||||||||||||||||||||||||||||||
| const ruleStatus = ( | ||||||||||||||||||||||||||||||||||||||||||||||
| username: string, | ||||||||||||||||||||||||||||||||||||||||||||||
| id: EmailAddressRuleId, | ||||||||||||||||||||||||||||||||||||||||||||||
| availability?: AddressAvailability, | ||||||||||||||||||||||||||||||||||||||||||||||
| ): EmailAddressRuleStatus => validateEmailAddress(username, availability).find((rule) => rule.id === id)!.status; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| describe('emailAddressRules', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| describe('length rule', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -179,14 +184,46 @@ describe('emailAddressRules', () => { | |||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| test('When username is not a reserved name, then it is valid', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| test('When username is well formatted but availability has not been checked yet, then it is idle', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const username = 'jane.doe'; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const status = ruleStatus(username, 'available'); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| expect(status).toBe('idle'); | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| test('When the availability check is in flight, then it is idle', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const status = ruleStatus('jane.doe', 'available', { status: 'checking' }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| expect(status).toBe('idle'); | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| test('When the backend reports the address as available, then it is valid', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const status = ruleStatus('jane.doe', 'available', { status: 'available' }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| expect(status).toBe('valid'); | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| test('When the backend reports the address as taken with a suggestion, then it is invalid and carries the suggestion', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const rule = validateEmailAddress('jane.doe', { status: 'taken', suggestion: 'jane.doe1@inxt.me' }).find( | ||||||||||||||||||||||||||||||||||||||||||||||
| (r) => r.id === 'available', | ||||||||||||||||||||||||||||||||||||||||||||||
| )!; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| expect(rule.status).toBe('invalid'); | ||||||||||||||||||||||||||||||||||||||||||||||
| expect(rule.labelKey).toBe('identitySetup.updateEmail.rules.taken'); | ||||||||||||||||||||||||||||||||||||||||||||||
| expect(rule.labelParams).toEqual({ suggestion: 'jane.doe1@inxt.me' }); | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| test('When the backend reports the address as taken without a suggestion, then it is invalid with the fallback label', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const rule = validateEmailAddress('jane.doe', { status: 'taken', suggestion: null }).find( | ||||||||||||||||||||||||||||||||||||||||||||||
| (r) => r.id === 'available', | ||||||||||||||||||||||||||||||||||||||||||||||
| )!; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| expect(rule.status).toBe('invalid'); | ||||||||||||||||||||||||||||||||||||||||||||||
| expect(rule.labelKey).toBe('identitySetup.updateEmail.rules.takenNoSuggestion'); | ||||||||||||||||||||||||||||||||||||||||||||||
| expect(rule.labelParams).toBeUndefined(); | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| test('When username is empty, then it is idle', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const username = ''; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -195,15 +232,27 @@ describe('emailAddressRules', () => { | |||||||||||||||||||||||||||||||||||||||||||||
| expect(status).toBe('idle'); | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| test('When a formatting rule fails, then it stays idle instead of being checked', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const username = 'Jane'; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const status = ruleStatus(username, 'available'); | ||||||||||||||||||||||||||||||||||||||||||||||
| test('When a formatting rule fails, then it stays idle even if the backend reported availability', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const status = ruleStatus('Jane', 'available', { status: 'available' }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| expect(status).toBe('idle'); | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| describe('isEmailAddressFormatValid', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| 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); | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+243
to
+252
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Separate Arrange, Act, and Assert. These tests invoke the subject inside 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
Suggested change
🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| describe('isRuleStatusValid', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| test('When status is valid, then it returns true', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const status: EmailAddressRuleStatus = 'valid'; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -231,18 +280,26 @@ describe('emailAddressRules', () => { | |||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| describe('isEmailAddressValid', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| test('When username satisfies every rule, then it is valid', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| test('When username satisfies every rule and the backend confirmed availability, then it is valid', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const username = 'jane.doe-99_x'; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const result = isEmailAddressValid(username); | ||||||||||||||||||||||||||||||||||||||||||||||
| const result = isEmailAddressValid(username, { status: 'available' }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| expect(result).toBe(true); | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| test('When availability has not been confirmed yet, then it is invalid', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const username = 'jane.doe-99_x'; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const result = isEmailAddressValid(username); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| expect(result).toBe(false); | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| test('When username fails at least one rule, then it is invalid', () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const username = 'admin'; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const result = isEmailAddressValid(username); | ||||||||||||||||||||||||||||||||||||||||||||||
| const result = isEmailAddressValid(username, { status: 'available' }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| expect(result).toBe(false); | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 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
Source: Coding guidelines