Skip to content
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

Add error-specific validation message in site identification page #91748

Merged
merged 20 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from 12 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
118 changes: 117 additions & 1 deletion client/blocks/import/capture/capture-input.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { recordTracksEvent } from '@automattic/calypso-analytics';
import { Button, FormLabel } from '@automattic/components';
import { useHasEnTranslation } from '@automattic/i18n-utils';
import { NextButton } from '@automattic/onboarding';
import { createElement, createInterpolateElement } from '@wordpress/element';
import { Icon, info } from '@wordpress/icons';
Expand Down Expand Up @@ -41,9 +42,11 @@ const CaptureInput: FunctionComponent< Props > = ( props ) => {
} = props;

const translate = useTranslate();
const hasEnTranslation = useHasEnTranslation();
const [ urlValue, setUrlValue ] = useState( '' );
const [ isValid, setIsValid ] = useState( false );
const [ submitted, setSubmitted ] = useState( false );
const [ validationMessage, setValidationMessage ] = useState( '' );
const lastInvalidValue = useRef< string | undefined >();
const showValidationMsg = hasError || ( submitted && ! isValid );
const { search } = useLocation();
Expand All @@ -69,13 +72,124 @@ const CaptureInput: FunctionComponent< Props > = ( props ) => {
}
}

function isIDN( url: string ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have this function outside the component body?
Each render is recreating the function.

Copy link
Contributor Author

@Imran92 Imran92 Jun 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've handled it here 228d668

try {
// Regex to extract the hostname from the URL.
const urlRegex = /^(?:https?:\/\/)?(?:www\.)?([^/]+)/i;
const match = url.match( urlRegex );

if ( ! match ) {
return false;
}

// Extract the hostname.
const hostname = match[ 1 ];

// Check for non-ASCII characters
for ( let i = 0; i < hostname.length; i++ ) {
if ( hostname.charCodeAt( i ) > 127 ) {
return true;
}
}

// Check if the hostname starts with the Punycode prefix.
if ( hostname.startsWith( 'xn--' ) ) {
return true;
}
} catch ( e ) {
return false;
}
}

function validateUrl( url: string ) {
const isValid = CAPTURE_URL_RGX.test( url );
setIsValid( isValid );
const tempValidationMessage = isValid ? '' : getValidationMessage( url );
setValidationMessage( tempValidationMessage );
}

// TODO: Just return the translated string directly from getValidationMessage once we have translations for all messages.
const errorMessages = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Move all error message logic to a private component, since is this pretty big.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, done here 228d668

'no-tld': {
message: translate(
'Your URL is missing a top-level domain (e.g., .com, .net, etc.). Example URL: example.com'
),
messageString:
'Your URL is missing a top-level domain (e.g., .com, .net, etc.). Example URL: example.com',
},
email: {
message: translate(
'It looks like you’ve entered an email address. Please enter a valid URL instead (e.g., example.com).'
),
messageString:
'It looks like you’ve entered an email address. Please enter a valid URL instead (e.g., example.com).',
},
'invalid-chars': {
message: translate(
'URL contains invalid characters. Please remove special characters and enter a valid URL (e.g., example.com).'
),
messageString:
'URL contains invalid characters. Please remove special characters and enter a valid URL (e.g., example.com).',
},
'invalid-protocol': {
message: translate(
'URLs with protocols can only start with http:// or https:// (e.g., https://example.com).'
),
messageString:
'URLs with protocols can only start with http:// or https:// (e.g., https://example.com).',
},
'idn-url': {
message: translate(
'Looks like you’ve entered an internationalized domain name (IDN). Please enter a standard URL instead (e.g., example.com).'
),
messageString:
'Looks like you’ve entered an internationalized domain name (IDN). Please enter a standard URL instead (e.g., example.com).',
},
default: {
message: translate(
'Please enter a valid website address (e.g., example.com). You can copy and paste.'
),
messageString:
'Please enter a valid website address (e.g., example.com). You can copy and paste.',
},
};

function getValidationMessage( url: string ) {
const hasEnTranslationForAllMessages = Object.values( errorMessages ).every( ( message ) =>
hasEnTranslation( message.messageString )
);

if ( ! hasEnTranslationForAllMessages ) {
return translate( 'Please enter a valid website address. You can copy and paste.' );
}

const missingTLDRegex = /^(?:https?:\/\/)?(?!.*\.[a-z]{2,})([a-zA-Z0-9-_]+)$/;
const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
const invalidURLProtocolRegex = /^(?!https?:\/\/)\S+:\/\//;
const invalidCharsRegex = /[^a-z0-9\-._~!$&'()*+,;:@/?=%[\]]/i;

const removedInitialDots = url.replace( 'www.', '' );

let errorMessage = errorMessages[ 'default' ].message;

if ( emailRegex.test( url ) ) {
errorMessage = errorMessages[ 'email' ].message;
} else if ( isIDN( url ) ) {
errorMessage = errorMessages[ 'idn-url' ].message;
} else if ( invalidCharsRegex.test( url ) ) {
errorMessage = errorMessages[ 'invalid-chars' ].message;
} else if ( missingTLDRegex.test( removedInitialDots ) ) {
errorMessage = errorMessages[ 'no-tld' ].message;
} else if ( invalidURLProtocolRegex.test( url ) ) {
errorMessage = errorMessages[ 'invalid-protocol' ].message;
}

return errorMessage;
}

function onChange( e: ChangeEvent< HTMLInputElement > ) {
const trimmedValue = e.target.value.trim();
setSubmitted( false );
setUrlValue( trimmedValue );
validateUrl( trimmedValue );
onInputChange?.( trimmedValue );
Expand Down Expand Up @@ -119,7 +233,9 @@ const CaptureInput: FunctionComponent< Props > = ( props ) => {
{ showValidationMsg && (
<>
<Icon icon={ info } size={ 20 } />{ ' ' }
{ translate( 'Please enter a valid website address. You can copy and paste.' ) }
{ validationMessage
? validationMessage
: translate( 'Please enter a valid website address. You can copy and paste.' ) }
</>
) }
</span>
Expand Down
145 changes: 145 additions & 0 deletions client/blocks/import/capture/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,149 @@ describe( 'CaptureInput', () => {

expect( screen.getByRole( 'button', { name: /XYZ/ } ) ).toBeInTheDocument();
} );

it( 'should show no TLD error', async () => {
const onInputEnter = jest.fn();
const { getByText } = render(
<MemoryRouter>
<CaptureInput onInputEnter={ onInputEnter } />
</MemoryRouter>
);

await userEvent.type( screen.getByLabelText( /Enter the URL of the site/ ), 'myblog' );

await userEvent.click( screen.getByRole( 'button', { name: /Continue/ } ) );

expect(
getByText(
'Your URL is missing a top-level domain (e.g., .com, .net, etc.). Example URL: example.com'
)
).toBeInTheDocument();
} );

it( 'should show no TLD error for URL with protocol', async () => {
const onInputEnter = jest.fn();
const { getByText } = render(
<MemoryRouter>
<CaptureInput onInputEnter={ onInputEnter } />
</MemoryRouter>
);

await userEvent.type( screen.getByLabelText( /Enter the URL of the site/ ), 'https://myblog' );

await userEvent.click( screen.getByRole( 'button', { name: /Continue/ } ) );

expect(
getByText(
'Your URL is missing a top-level domain (e.g., .com, .net, etc.). Example URL: example.com'
)
).toBeInTheDocument();
} );
} );

// Helper function to enter URL and click CTA.
const enterUrlAndContinue = async ( url ) => {
await userEvent.type( screen.getByLabelText( /Enter the URL of the site/ ), url );
await userEvent.click( screen.getByRole( 'button', { name: /Continue/ } ) );
};

describe( 'URL Validation', () => {
beforeEach( () => {
const onInputEnter = jest.fn();
render(
<MemoryRouter>
<CaptureInput onInputEnter={ onInputEnter } />
</MemoryRouter>
);
} );

test( 'should show error for missing top-level domain', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually, we use the it instead of test the file already has scenarios with it.

Suggested change
test( 'should show error for missing top-level domain', async () => {
it.( 'shows error for missing top-level domain', async () => {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done under it.each here 3447cd3

await enterUrlAndContinue( 'myblog' );

expect(
screen.getByText(
'Your URL is missing a top-level domain (e.g., .com, .net, etc.). Example URL: example.com'
)
).toBeInTheDocument();
} );

test( 'should show error for missing top-level domain event when protocol is present', async () => {
await enterUrlAndContinue( 'https://myblog' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use dinamic to test all scenarios:

Example:

it.each([
  {scenario: 'A', input: 'XXX',  error: 'YYY'},
  {scenario: 'B', input: 'BB',  error: 'ZZZ'},
])(' show error ', ({scenario,  input, expected}) => {
    await enterUrlAndContinue( input );
expect(
			screen.getByText( error )
		).toBeInTheDocument();
});

Docs here https://jestjs.io/docs/api#testeachtablename-fn-timeout

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good suggestion, thank you! Done here 3447cd3


expect(
screen.getByText(
'Your URL is missing a top-level domain (e.g., .com, .net, etc.). Example URL: example.com'
)
).toBeInTheDocument();
} );

test( 'should show error for email instead of URL', async () => {
await enterUrlAndContinue( 'user@example.com' );

expect(
screen.getByText(
'It looks like you’ve entered an email address. Please enter a valid URL instead (e.g., example.com).'
)
).toBeInTheDocument();
} );

test( 'should show error for URL with invalid characters', async () => {
await enterUrlAndContinue( 'http://my^blog.com' );

expect(
screen.getByText(
'URL contains invalid characters. Please remove special characters and enter a valid URL (e.g., example.com).'
)
).toBeInTheDocument();
} );

test( 'should show error for invalid protocol', async () => {
await enterUrlAndContinue( 'ftp://example.com' );

expect(
screen.getByText(
'URLs with protocols can only start with http:// or https:// (e.g., https://example.com).'
)
).toBeInTheDocument();
} );

test( 'should show error for invalid protocol for filepath', async () => {
await enterUrlAndContinue( 'file:///C:/DEVELOPER/index.html' );

expect(
screen.getByText(
'URLs with protocols can only start with http:// or https:// (e.g., https://example.com).'
)
).toBeInTheDocument();
} );

test( 'should show IDN error message for punycode IDNs', async () => {
await enterUrlAndContinue( 'https://xn--example.com' );

expect(
screen.getByText(
'Looks like you’ve entered an internationalized domain name (IDN). Please enter a standard URL instead (e.g., example.com).'
)
).toBeInTheDocument();
} );

test( 'should show IDN error message for unicode IDNs', async () => {
await enterUrlAndContinue( 'www.例子.测试' );

expect(
screen.getByText(
'Looks like you’ve entered an internationalized domain name (IDN). Please enter a standard URL instead (e.g., example.com).'
)
).toBeInTheDocument();
} );

test( 'should show default error message', async () => {
await enterUrlAndContinue( 'example.com-' );

expect(
screen.getByText(
'Please enter a valid website address (e.g., example.com). You can copy and paste.'
)
).toBeInTheDocument();
} );
} );
Loading