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

Refactor credentials from error handling #94682

Open
wants to merge 19 commits into
base: trunk
Choose a base branch
from
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 @@ -14,7 +14,7 @@ export const AccessMethodPicker: FC< CredentialsFormFieldProps > = ( { control }
<div className="site-migration-credentials__radio">
<Controller
control={ control }
name="howToAccessSite"
name="migrationType"
defaultValue="credentials"
render={ ( { field: { value, ...props } } ) => (
<FormRadio
Expand All @@ -32,7 +32,7 @@ export const AccessMethodPicker: FC< CredentialsFormFieldProps > = ( { control }
<div className="site-migration-credentials__radio">
<Controller
control={ control }
name="howToAccessSite"
name="migrationType"
defaultValue="backup"
render={ ( { field: { value, onBlur, ...props } } ) => (
<FormRadio
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,20 @@ export const SiteAddressField: React.FC< Props > = ( {

return (
<div className="site-migration-credentials__form-field">
<FormLabel htmlFor="site-address">
<FormLabel htmlFor="from_url">
{ isEnglishLocale ? translate( 'Current site address' ) : translate( 'Site address' ) }
</FormLabel>
<Controller
control={ control }
name="siteAddress"
name="from_url"
rules={ {
required: translate( 'Please enter your WordPress site address.' ),
validate: validateSiteAddress,
} }
render={ ( { field } ) => (
<FormTextInput
id="site-address"
isError={ !! errors?.siteAddress }
id="from_url"
isError={ !! errors?.from_url }
placeholder={ placeholder }
readOnly={ !! importSiteQueryParam }
disabled={ !! importSiteQueryParam }
Expand All @@ -56,7 +56,7 @@ export const SiteAddressField: React.FC< Props > = ( {
/>
) }
/>
<ErrorMessage error={ errors?.siteAddress } />
<ErrorMessage error={ errors?.from_url } />
</div>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { useTranslate } from 'i18n-calypso';
import { useCallback, useMemo } from 'react';
import { FieldErrors } from 'react-hook-form';
import { ApiError, CredentialsFormData } from '../types';

// This function is used to map the error message to the correct field in the form.
// Backend is returning the errors related to backup files using 'from_url' key
// but we need to use 'backupFileLocation' to identify the field in the form.
const getFieldName = ( key: string, migrationType: string ) => {
return 'backup' === migrationType && key === 'from_url' ? 'backupFileLocation' : key;
};

// ** This hook is used to map the error messages to the form fields errors.
export const useFormErrorMapping = (
error?: ApiError | null,
variables?: CredentialsFormData | null
): FieldErrors< CredentialsFormData > | undefined => {
const translate = useTranslate();

const fieldMapping: Record< string, { type: string; message: string } | null > = useMemo(
() => ( {
from_url: { type: 'manual', message: translate( 'Enter a valid URL.' ) },
username: { type: 'manual', message: translate( 'Enter a valid username.' ) },
password: { type: 'manual', message: translate( 'Enter a valid password.' ) },
backupFileLocation: { type: 'manual', message: translate( 'Enter a valid URL.' ) },
} ),
[ translate ]
);

const getTranslatedMessage = useCallback(
( key: string ) => {
return (
fieldMapping[ key ] ?? {
type: 'manual',
message: translate( 'Invalid input, please check again' ),
}
);
},
[ fieldMapping, translate ]
);

const handleServerError = useCallback(
( error: ApiError, { migrationType }: CredentialsFormData ) => {
const { code, message, data } = error;

if ( code === 'rest_missing_callback_param' || ! code ) {
return {
root: {
type: 'manual',
message: translate( 'An error occurred while saving credentials.' ),
},
};
}

if ( code !== 'rest_invalid_param' || ! data?.params ) {
return { root: { type: 'manual', message } };
}

const invalidFields = Object.keys( data.params );

return invalidFields.reduce(
( errors, key ) => {
const fieldName = getFieldName( key, migrationType );
const message = getTranslatedMessage( key );

errors[ fieldName ] = message;
return errors;
},
{} as Record< string, { type: string; message: string } >
);
},
[ getTranslatedMessage, translate ]
);

return useMemo( () => {
if ( error && variables ) {
return handleServerError( error, variables ) as FieldErrors< CredentialsFormData >;
}
return undefined;
}, [ error, handleServerError, variables ] );
};
Original file line number Diff line number Diff line change
Expand Up @@ -128,21 +128,28 @@ describe( 'SiteMigrationCredentials', () => {
expect( wpcomRequest ).not.toHaveBeenCalled();
} );

it( 'shows errors on the required fields when the user does not fill the fields', async () => {
it( 'shows errors on the required fields when the user does not fill the fields when user select credentials option', async () => {
render();
await userEvent.click( continueButton() );
await userEvent.click( credentialsOption() );
expect( getByText( messages.urlError ) ).toBeVisible();
expect( getByText( messages.usernameError ) ).toBeVisible();
expect( getByText( messages.passwordError ) ).toBeVisible();
} );

expect( getByText( messages.urlError ) ).toBeInTheDocument();
expect( getByText( messages.usernameError ) ).toBeInTheDocument();
expect( getByText( messages.passwordError ) ).toBeInTheDocument();
it( 'shows errors on the required fields when the user does not fill the fields when user select backup option', async () => {
render();
await userEvent.click( backupOption() );
await userEvent.click( continueButton() );
expect( getByText( /Please enter a valid URL/ ) ).toBeVisible();
} );

it( 'shows error when user set invalid site address', async () => {
render();
await userEvent.type( siteAddressInput(), 'invalid-site-address' );
await userEvent.click( continueButton() );

expect( getByText( messages.noTLDError ) ).toBeInTheDocument();
expect( getByText( messages.noTLDError ) ).toBeVisible();
} );

it( 'fills the site address and disable it when the user already informed the site address on previous step', async () => {
Expand Down Expand Up @@ -214,10 +221,26 @@ describe( 'SiteMigrationCredentials', () => {
await fillAllFields();
await userEvent.click( continueButton() );

expect( getByText( /Error message from backend/ ) ).toBeVisible();
await waitFor( () => {
expect( getByText( /Error message from backend/ ) ).toBeVisible();
} );
expect( submit ).not.toHaveBeenCalled();
} );

it( 'shows an generic error when server doesn`t return error', async () => {
const submit = jest.fn();
render( { navigation: { submit } } );

( wpcomRequest as jest.Mock ).mockRejectedValue( {} );

await fillAllFields();
await userEvent.click( continueButton() );

await waitFor( () => {
expect( getByText( /An error occurred while saving credentials./ ) ).toBeVisible();
} );
} );

it( 'shows a notice when URL contains error=ticket-creation', async () => {
const submit = jest.fn();
const initialEntry = '/site-migration-credentials?error=ticket-creation';
Expand All @@ -227,6 +250,9 @@ describe( 'SiteMigrationCredentials', () => {
const errorMessage = await findByText(
/We ran into a problem submitting your details. Please try again shortly./
);
expect( errorMessage ).toBeVisible();

await waitFor( () => {
expect( errorMessage ).toBeVisible();
} );
} );
} );
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
import { Control, FieldErrors } from 'react-hook-form';

export interface CredentialsFormData {
from_url: string;
username: string;
password: string;
backupFileLocation: string;
migrationType: 'credentials' | 'backup';
notes: string;
}

export interface ApiFormData {
siteAddress: string;
username: string;
password: string;
Expand All @@ -9,6 +18,14 @@ export interface CredentialsFormData {
howToAccessSite: 'credentials' | 'backup';
}

export interface ApiError {
code: string;
message: string;
data: {
params?: Record< string, string >;
};
}

export interface CredentialsFormFieldProps {
control: Control< CredentialsFormData >;
errors?: FieldErrors< CredentialsFormData >;
Expand All @@ -23,5 +40,4 @@ export interface MigrationError {
params?: Record< string, string >;
};
};
status: number;
}
Loading
Loading