Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const EmailAddressRulesPanel = ({ rules }: EmailAddressRulesPanelProps) =
return (
<li key={rule.id} className={`flex items-center gap-2 text-sm ${STATUS_TEXT_CLASSNAME[rule.status]}`}>
<StatusIcon size={16} weight="fill" className="shrink-0" />
{translate(rule.labelKey)}
{translate(rule.labelKey, rule.labelParams)}
</li>
);
})}
Expand Down
7 changes: 4 additions & 3 deletions src/features/identity-setup/components/UpdateEmail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ export const UpdateEmail = ({ userFullName, activeDomains, currentEmail, onNext
const {
username,
rules,
isValid,
canSubmit,
isSubmitting,
validateAddress,
domain,
setDomain,
Expand All @@ -36,7 +37,7 @@ export const UpdateEmail = ({ userFullName, activeDomains, currentEmail, onNext
if (e.key !== 'Enter' || (e.target as HTMLElement).tagName === 'BUTTON') return;

e.preventDefault();
submit();
void submit();
};

return (
Expand Down Expand Up @@ -85,7 +86,7 @@ export const UpdateEmail = ({ userFullName, activeDomains, currentEmail, onNext
</div>

<div className="flex flex-col w-full">
<Button type="submit" disabled={!isValid}>
<Button type="submit" disabled={!canSubmit || isSubmitting} loading={isSubmitting}>
{translate('identitySetup.updateEmail.action')}
</Button>
</div>
Expand Down
77 changes: 67 additions & 10 deletions src/features/identity-setup/hooks/emailAddressRules.test.ts
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', () => {
Expand Down Expand Up @@ -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 = '';

Expand All @@ -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', () => {

Copy link
Copy Markdown

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
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);
Comment on lines +243 to +252

Copy link
Copy Markdown

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

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.

Suggested change
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('isRuleStatusValid', () => {
test('When status is valid, then it returns true', () => {
const status: EmailAddressRuleStatus = 'valid';
Expand Down Expand Up @@ -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);
});
Expand Down
59 changes: 49 additions & 10 deletions src/features/identity-setup/hooks/emailAddressRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,18 @@ const RESERVED_USERNAMES = new Set([
export type EmailAddressRuleId = 'length' | 'charset' | 'edges' | 'available';
export type EmailAddressRuleStatus = 'idle' | 'valid' | 'invalid';

export type AddressAvailability =
| { status: 'unknown' }
| { status: 'checking' }
| { status: 'available' }
| { status: 'taken'; suggestion: string | null };

export const UNKNOWN_AVAILABILITY: AddressAvailability = { status: 'unknown' };

export interface EmailAddressRule {
id: EmailAddressRuleId;
labelKey: TranslationKey;
labelParams?: Record<string, string>;
status: EmailAddressRuleStatus;
}

Expand All @@ -49,14 +58,48 @@ const isCharsetValid = (username: string): boolean =>

const isEdgesValid = (username: string): boolean => !EDGE_SPECIAL_CHARS_REGEX.test(username);

const isAvailable = (username: string): boolean => !RESERVED_USERNAMES.has(username);
const isReserved = (username: string): boolean => RESERVED_USERNAMES.has(username);

export const isEmailAddressFormatValid = (username: string): boolean =>
isLengthValid(username) && isCharsetValid(username) && isEdgesValid(username) && !isReserved(username);

const availabilityRule = (
username: string,
formatValid: boolean,
availability: AddressAvailability,
): EmailAddressRule => {
const base = { id: 'available', labelKey: 'identitySetup.updateEmail.rules.available' } as const;

if (!formatValid) {
return { ...base, status: isReserved(username) ? 'invalid' : 'idle' };
}

export const validateEmailAddress = (username: string): EmailAddressRule[] => {
switch (availability.status) {
case 'available':
return { ...base, status: 'valid' };
case 'taken':
return availability.suggestion
? {
...base,
labelKey: 'identitySetup.updateEmail.rules.taken',
labelParams: { suggestion: availability.suggestion },
status: 'invalid',
}
: { ...base, labelKey: 'identitySetup.updateEmail.rules.takenNoSuggestion', status: 'invalid' };
default:
return { ...base, status: 'idle' };
}
};

export const validateEmailAddress = (
username: string,
availability: AddressAvailability = UNKNOWN_AVAILABILITY,
): EmailAddressRule[] => {
const hasContent = username.length > 0;
const lengthValid = isLengthValid(username);
const charsetValid = isCharsetValid(username);
const edgesValid = hasContent && isEdgesValid(username);
const formatValid = lengthValid && charsetValid && edgesValid;
const formatValid = lengthValid && charsetValid && edgesValid && !isReserved(username);

return [
{
Expand All @@ -74,15 +117,11 @@ export const validateEmailAddress = (username: string): EmailAddressRule[] => {
labelKey: 'identitySetup.updateEmail.rules.edges',
status: hasContent ? (edgesValid ? 'valid' : 'invalid') : 'idle',
},
{
id: 'available',
labelKey: 'identitySetup.updateEmail.rules.available',
status: formatValid ? (isAvailable(username) ? 'valid' : 'invalid') : 'idle',
},
availabilityRule(username, formatValid, availability),
];
};

export const isRuleStatusValid = (status: EmailAddressRuleStatus): boolean => status === 'valid';

export const isEmailAddressValid = (username: string): boolean =>
validateEmailAddress(username).every((rule) => isRuleStatusValid(rule.status));
export const isEmailAddressValid = (username: string, availability?: AddressAvailability): boolean =>
validateEmailAddress(username, availability).every((rule) => isRuleStatusValid(rule.status));
Loading
Loading