From 9cf625bcf9b727ca5b8255e39a79b835d6cd868c Mon Sep 17 00:00:00 2001 From: Gabriel Caires Date: Tue, 17 Sep 2024 17:10:57 +0100 Subject: [PATCH 01/15] Fix typo on file name --- .../components/{crdentials-form.tsx => credentials-form.tsx} | 0 .../steps-repository/site-migration-credentials/index.tsx | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/{crdentials-form.tsx => credentials-form.tsx} (100%) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/crdentials-form.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/credentials-form.tsx similarity index 100% rename from client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/crdentials-form.tsx rename to client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/credentials-form.tsx diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/index.tsx index 222bb8a61c5b8..c6f73489879a1 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/index.tsx @@ -3,7 +3,7 @@ import { useTranslate } from 'i18n-calypso'; import DocumentHead from 'calypso/components/data/document-head'; import FormattedHeader from 'calypso/components/formatted-header'; import { recordTracksEvent } from 'calypso/lib/analytics/tracks'; -import { CredentialsForm } from './components/crdentials-form'; +import { CredentialsForm } from './components/credentials-form'; import type { Step } from '../../types'; import './style.scss'; From 2597b9d9bbfb85fe6832945779c56fbfa1c49758 Mon Sep 17 00:00:00 2001 From: Gabriel Caires Date: Wed, 18 Sep 2024 12:03:22 +0100 Subject: [PATCH 02/15] Improve credentials test suite --- .../site-migration-credentials/test/index.tsx | 451 ++++++------------ 1 file changed, 156 insertions(+), 295 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx index 14332439800c2..17bd477403bc9 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx @@ -29,319 +29,180 @@ const messages = { "Looks like your site address is missing its domain extension. Please try again with something like 'example.com' or 'example.net'.", }; +const continueButton = () => screen.getByRole( 'button', { name: /Continue/ } ); +const siteAddressInput = () => screen.getByLabelText( 'Site address' ); +const usernameInput = () => screen.getByLabelText( 'WordPress admin username' ); +const passwordInput = () => screen.getByLabelText( 'Password' ); +const backupOption = () => screen.getByRole( 'radio', { name: 'Backup file' } ); +const credentialsOption = () => screen.getByRole( 'radio', { name: 'WordPress credentials' } ); +const backupFileInput = () => screen.getByLabelText( 'Backup file location' ); +//TODO: it requires a testid because there is no accessible name, it is an issue with the component +const specialInstructionsInput = () => screen.getByTestId( 'special-instructions-textarea' ); +const specialInstructionsButton = () => + screen.getByRole( 'button', { name: 'Special instructions' } ); +const skipButton = () => + screen.getByRole( 'button', { name: /Skip, I need help providing access/ } ); + +const fillAllFields = async () => { + await userEvent.click( credentialsOption() ); + await userEvent.type( siteAddressInput(), 'site-url.com' ); + await userEvent.type( usernameInput(), 'username' ); + await userEvent.type( passwordInput(), 'password' ); +}; + describe( 'SiteMigrationCredentials', () => { beforeAll( () => nock.disableNetConnect() ); + beforeEach( () => { + jest.clearAllMocks(); + } ); - it( 'renders the form', async () => { + it( 'creates a credentials ticket', async () => { const submit = jest.fn(); render( { navigation: { submit } } ); - expect( screen.queryByRole( 'button', { name: /Continue/ } ) ).toBeInTheDocument(); - expect( screen.queryByText( 'How can we access your site?' ) ).toBeInTheDocument(); - expect( screen.queryByText( 'Site address' ) ).toBeInTheDocument(); - expect( screen.queryByText( 'WordPress admin username' ) ).toBeInTheDocument(); - expect( screen.queryByText( 'Password' ) ).toBeInTheDocument(); - expect( screen.queryByText( 'Special instructions' ) ).toBeInTheDocument(); + await userEvent.click( credentialsOption() ); + await userEvent.type( siteAddressInput(), 'site-url.com' ); + await userEvent.type( usernameInput(), 'username' ); + await userEvent.type( passwordInput(), 'password' ); + + await userEvent.click( specialInstructionsButton() ); + await userEvent.type( specialInstructionsInput(), 'notes' ); + await userEvent.click( continueButton() ); + + expect( wpcomRequest ).toHaveBeenCalledWith( { + path: 'sites/site-url.wordpress.com/automated-migration', + apiNamespace: 'wpcom/v2/', + apiVersion: '2', + method: 'POST', + body: { + migration_type: 'credentials', + blog_url: 'site-url.wordpress.com', + notes: 'notes', + from_url: 'site-url.com', + username: 'username', + password: 'password', + }, + } ); + + await waitFor( () => { + expect( submit ).toHaveBeenCalled(); + } ); } ); - it( 'does not show any error message by default', async () => { + it( 'skips the credential creation when the user does not fill the fields', async () => { const submit = jest.fn(); render( { navigation: { submit } } ); - expect( screen.queryByText( messages.urlError ) ).not.toBeInTheDocument(); - expect( screen.queryByText( messages.usernameError ) ).not.toBeInTheDocument(); + await userEvent.click( skipButton() ); + + expect( submit ).toHaveBeenCalledWith( { action: 'skip' } ); + expect( wpcomRequest ).not.toHaveBeenCalled(); } ); - it.each( [ - [ - 'site-url.com', - 'username', - 'password', - [], - [ messages.passwordError, messages.urlError, messages.usernameError ], - ], - [ - 'siteurlnotld', - 'username', - 'password', - [ messages.noTLDError ], - [ messages.passwordError, messages.urlError, messages.usernameError ], - ], - [ - '', - 'username', - 'password', - [ messages.urlError ], - [ messages.passwordError, messages.usernameError ], - ], - [ - '', - '', - 'password', - [ messages.urlError, messages.usernameError ], - [ messages.passwordError ], - ], - [ - '', - 'username', - '', - [ messages.urlError, messages.passwordError ], - [ messages.usernameError ], - ], - [ '', ' ', '', [ messages.urlError, messages.usernameError ], [] ], - [ - 'backup-url.com', - 'username', - '', - [ messages.passwordError ], - [ messages.usernameError, messages.urlError ], - ], - [ - 'backup-url.com', - '', - '', - [ messages.usernameError, messages.passwordError ], - [ messages.urlError ], - ], - [ '', '', '', [ messages.usernameError, messages.urlError, messages.passwordError ], [] ], - ] )( - 'shows correct error messages for url:%s--username:%s--pass:%s--', - async ( siteAddress, username, password, expectedErrors, notExpectedErrors ) => { - const submit = jest.fn(); - render( { navigation: { submit } } ); - - const expandNotesButton = screen.getByTestId( 'special-instructions' ); - await userEvent.click( expandNotesButton ); - - const addressInput = screen.getByLabelText( /Site address/ ); - const usernameInput = screen.getByLabelText( /WordPress admin username/ ); - const passwordInput = screen.getByLabelText( /Password/ ); - const notesInput = screen.getByTestId( 'special-instructions-textarea' ); - - siteAddress && ( await userEvent.type( addressInput, siteAddress ) ); - username && ( await userEvent.type( usernameInput, username ) ); - password && ( await userEvent.type( passwordInput, password ) ); - await userEvent.type( notesInput, 'notes' ); - - await waitFor( async () => { - await userEvent.click( screen.getByRole( 'button', { name: /Continue/ } ) ); - } ); - - expectedErrors.forEach( ( text ) => { - expect( screen.queryByText( text ) ).toBeInTheDocument(); - } ); - - notExpectedErrors.forEach( ( text ) => { - expect( screen.queryByText( text ) ).not.toBeInTheDocument(); - } ); - } - ); - - it.each( [ - [ - 'credentials', - 'site-url.com', - 'username', - 'password', - [], - [ messages.passwordError, messages.urlError, messages.usernameError ], - ], - [ - 'backup', - 'backupsite-url.com', - '', - '', - [], - [ messages.passwordError, messages.urlError, messages.usernameError ], - ], - ] )( - 'sends correctly formed request', - async ( - howToAccessSite, - siteAddress, - username, - password, - expectedErrors, - notExpectedErrors - ) => { - ( wpcomRequest as jest.Mock ).mockClear(); - const submit = jest.fn(); - render( { navigation: { submit } } ); - - const isBackupRequest = howToAccessSite === 'backup'; - - if ( isBackupRequest ) { - const backupRadio = screen.getByLabelText( 'Backup file' ); - await userEvent.click( backupRadio ); - } - - const addressInput = screen.getByLabelText( - isBackupRequest ? /Backup file location/ : /Site address/ - ); - - const expandNotesButton = screen.getByTestId( 'special-instructions' ); - await userEvent.click( expandNotesButton ); - - const notesInput = screen.getByTestId( 'special-instructions-textarea' ); - - await userEvent.type( addressInput, siteAddress ); - username && - ( await userEvent.type( screen.getByLabelText( /WordPress admin username/ ), username ) ); - password && ( await userEvent.type( screen.getByLabelText( /Password/ ), password ) ); - await userEvent.type( notesInput, 'notes' ); - - await waitFor( async () => { - await userEvent.click( screen.getByRole( 'button', { name: /Continue/ } ) ); - } ); - - await waitFor( async () => { - expect( wpcomRequest ).toHaveBeenCalledWith( { - path: 'sites/site-url.wordpress.com/automated-migration', - apiNamespace: 'wpcom/v2/', - apiVersion: '2', - method: 'POST', - body: { - migration_type: howToAccessSite, - blog_url: 'site-url.wordpress.com', - notes: 'notes', - from_url: siteAddress, - ...( isBackupRequest - ? {} - : { - username, - password, - } ), - }, - } ); - } ); - - expectedErrors.forEach( ( text ) => { - expect( screen.queryByText( text ) ).toBeInTheDocument(); - } ); - notExpectedErrors.forEach( ( text ) => { - expect( screen.queryByText( text ) ).not.toBeInTheDocument(); - } ); - } - ); - - it.each( [ - [ - 'Enter a valid URL.', - { - code: 'rest_invalid_param', - message: 'Test', - data: { params: { from_url: 'Invalid Param' } }, - }, - ], - [ - 'Enter a valid username.', - { - code: 'rest_invalid_param', - message: 'Test', - data: { params: { username: 'Invalid Param' } }, - }, - ], - [ - 'Invalid input, please check again', - { - code: 'rest_invalid_param', - message: 'Test note', - data: { params: { notes: 'Invalid Param' } }, - }, - ], - [ - 'Enter a valid password.', - { - code: 'rest_invalid_param', - message: 'Test', - data: { params: { password: 'Invalid Param' } }, - }, - ], - [ - 'An error occurred while saving credentials.', - { - code: 'rest_invalid_param', - message: 'Test', - data: { params: { nonexistant: 'Invalid Param' } }, - }, - ], - [ - 'A test message', - { - code: 'rest_other_error', - message: 'A test message', - data: {}, - }, - ], - [ - 'An error occurred while saving credentials.', - { - code: 'rest_other_error_no_message', - data: {}, + it( 'creates a credentials using backup file', async () => { + const submit = jest.fn(); + render( { navigation: { submit } } ); + + await userEvent.click( backupOption() ); + await userEvent.type( backupFileInput(), 'backup-file.zip' ); + await userEvent.click( specialInstructionsButton() ); + await userEvent.type( specialInstructionsInput(), 'notes' ); + await userEvent.click( continueButton() ); + + //TODO: Ideally we should use nock to mock the request, but it is not working with the current implementation due to wpcomRequest usage that is well captured by nock. + expect( wpcomRequest ).toHaveBeenCalledWith( { + path: 'sites/site-url.wordpress.com/automated-migration', + apiNamespace: 'wpcom/v2/', + apiVersion: '2', + method: 'POST', + body: { + blog_url: 'site-url.wordpress.com', + migration_type: 'backup', + notes: 'notes', + from_url: 'backup-file.zip', }, - ], - ] )( - 'shows correct error messages after server validation -- %s -- %p', - async ( message, errorResponse ) => { - ( wpcomRequest as jest.Mock ).mockClear(); - - const submit = jest.fn(); - render( { navigation: { submit } } ); - - const expandNotesButton = screen.getByTestId( 'special-instructions' ); - await userEvent.click( expandNotesButton ); - - await userEvent.type( screen.getByLabelText( /Site address/ ), 'test.com' ); - await userEvent.type( screen.getByLabelText( /WordPress admin username/ ), 'username' ); - await userEvent.type( screen.getByLabelText( /Password/ ), 'password' ); - await userEvent.type( screen.getByTestId( 'special-instructions-textarea' ), 'notes' ); - - ( wpcomRequest as jest.Mock ).mockRejectedValue( errorResponse ); - - await waitFor( async () => { - await userEvent.click( screen.getByRole( 'button', { name: /Continue/ } ) ); - } ); - - await waitFor( async () => { - expect( wpcomRequest ).toHaveBeenCalledWith( { - path: 'sites/site-url.wordpress.com/automated-migration', - apiNamespace: 'wpcom/v2/', - apiVersion: '2', - method: 'POST', - body: { - migration_type: 'credentials', - blog_url: 'site-url.wordpress.com', - notes: 'notes', - from_url: 'test.com', - username: 'username', - password: 'password', - }, - } ); - } ); - - await waitFor( () => { - expect( screen.queryByText( message ) ).toBeInTheDocument(); - } ); - } - ); + } ); - it( 'shows a notice when URL contains error=ticket-creation', async () => { + await waitFor( () => { + expect( submit ).toHaveBeenCalled(); + } ); + } ); + + it( 'shows errors on the required fields when the user does not fill the fields', async () => { + render(); + await userEvent.click( continueButton() ); + + expect( screen.getByText( messages.urlError ) ).toBeInTheDocument(); + expect( screen.getByText( messages.usernameError ) ).toBeInTheDocument(); + expect( screen.getByText( messages.passwordError ) ).toBeInTheDocument(); + } ); + + it( 'shows error when user set invalid site address', async () => { + render(); + await userEvent.type( siteAddressInput(), 'invalid-site-address' ); + await userEvent.click( continueButton() ); + + expect( screen.getByText( messages.noTLDError ) ).toBeInTheDocument(); + } ); + + it( 'fills the site address and disable it when the user already informed the site address on previous step', async () => { + const initialEntry = '/site-migration-credentials?from=https://example.com'; + render( {}, { initialEntry } ); + + expect( siteAddressInput() ).toHaveValue( 'https://example.com' ); + expect( siteAddressInput() ).toBeDisabled(); + } ); + + it( 'shows error messages by each field when the server returns "invalid param" by each field', async () => { const submit = jest.fn(); - render( - { - navigation: { submit }, + render( { navigation: { submit } } ); + + ( wpcomRequest as jest.Mock ).mockRejectedValue( { + code: 'rest_invalid_param', + data: { + params: { + from_url: 'Invalid Param', + username: 'Invalid Param', + password: 'Invalid Param', + notes: 'Invalid Param', + }, }, - { - initialEntry: - '/site-migration-credentials?siteId=123&siteSlug=test.wordpress.com&error=ticket-creation', - } - ); + } ); - await waitFor( () => { - const errorNotice = screen.getByText( - 'We ran into a problem submitting your details. Please try again shortly.' - ); - expect( errorNotice ).toBeInTheDocument(); + await fillAllFields(); + await userEvent.click( continueButton() ); + + expect( screen.getByText( /Enter a valid URL/ ) ).toBeVisible(); + expect( screen.getByText( /Enter a valid username/ ) ).toBeVisible(); + expect( screen.getByText( /Enter a valid password/ ) ).toBeVisible(); + expect( submit ).not.toHaveBeenCalled(); + } ); + + it( 'shows an error message when the server returns a generic error', async () => { + const submit = jest.fn(); + render( { navigation: { submit } } ); + + ( wpcomRequest as jest.Mock ).mockRejectedValue( { + code: 'rest_other_error', + message: 'Error message from backend', } ); + + await fillAllFields(); + await userEvent.click( continueButton() ); + + expect( screen.getByText( /Error message from backend/ ) ).toBeVisible(); + expect( submit ).not.toHaveBeenCalled(); + } ); + + it( 'shows a notice when URL contains error=ticket-creation', async () => { + const submit = jest.fn(); + const initialEntry = '/site-migration-credentials?error=ticket-creation'; + + render( { navigation: { submit } }, { initialEntry } ); + + const errorMessage = await screen.findByText( + /We ran into a problem submitting your details. Please try again shortly./ + ); + expect( errorMessage ).toBeVisible(); } ); } ); From f7548979e6b76b94776ec7814fabd2ddbeaef804 Mon Sep 17 00:00:00 2001 From: Gabriel Caires Date: Wed, 18 Sep 2024 16:56:49 +0100 Subject: [PATCH 03/15] Add scenario when there is an error with the file upload url --- .../site-migration-credentials/test/index.tsx | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx index 17bd477403bc9..0d633d4d88619 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx @@ -171,11 +171,35 @@ describe( 'SiteMigrationCredentials', () => { await fillAllFields(); await userEvent.click( continueButton() ); + await waitFor( () => { + expect( screen.getByText( /Enter a valid URL/ ) ).toBeVisible(); + expect( screen.getByText( /Enter a valid username/ ) ).toBeVisible(); + expect( screen.getByText( /Enter a valid password/ ) ).toBeVisible(); + expect( submit ).not.toHaveBeenCalled(); + } ); + } ); - expect( screen.getByText( /Enter a valid URL/ ) ).toBeVisible(); - expect( screen.getByText( /Enter a valid username/ ) ).toBeVisible(); - expect( screen.getByText( /Enter a valid password/ ) ).toBeVisible(); - expect( submit ).not.toHaveBeenCalled(); + it( 'shows error message when there is an error on the with the backup file', async () => { + const submit = jest.fn(); + render( { navigation: { submit } } ); + + ( wpcomRequest as jest.Mock ).mockRejectedValue( { + code: 'rest_invalid_param', + data: { + params: { + from_url: 'Invalid Param', + }, + }, + } ); + + await userEvent.click( backupOption() ); + await userEvent.type( backupFileInput(), 'backup-file.zip' ); + await userEvent.click( continueButton() ); + + await waitFor( () => { + expect( screen.getByText( /Enter a valid URL/ ) ).toBeVisible(); + expect( submit ).not.toHaveBeenCalled(); + } ); } ); it( 'shows an error message when the server returns a generic error', async () => { From f916cb64f71e4c062f9c33341fbae28bc4039312 Mon Sep 17 00:00:00 2001 From: Gabriel Caires Date: Wed, 18 Sep 2024 17:22:04 +0100 Subject: [PATCH 04/15] Reorganize code to remove warnings --- .../components/backup-file-field.tsx | 8 +- .../components/credentials-form.tsx | 14 +- .../components/error-message.tsx | 8 +- .../components/password-field.tsx | 8 +- .../components/site-address-field.tsx | 12 +- .../components/special-instructions.tsx | 6 +- .../components/username-field.tsx | 8 +- .../use-credentials-form.ts | 156 ++++++++++-------- ...se-site-migration-credentials-mutation.tsx | 7 +- 9 files changed, 117 insertions(+), 110 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx index a055a9bcc7b97..14de5c706fb63 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx @@ -8,10 +8,10 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - errors: any; + error?: string; } -export const BackupFileField: React.FC< Props > = ( { control, errors } ) => { +export const BackupFileField: React.FC< Props > = ( { control, error } ) => { const translate = useTranslate(); const isBackupFileLocationValid = ( fileLocation: string ) => { @@ -34,14 +34,14 @@ export const BackupFileField: React.FC< Props > = ( { control, errors } ) => { ) } /> - +
{ translate( "Upload your file to a service like Dropbox or Google Drive to get a link. Don't forget to make sure that anyone with the link can access it." diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/credentials-form.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/credentials-form.tsx index 08eab372d557b..6d06719273616 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/credentials-form.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/credentials-form.tsx @@ -54,20 +54,22 @@ export const CredentialsForm: FC< CredentialsFormProps > = ( { onSubmit, onSkip
- - + +
) } - { accessMethod === 'backup' && } + { accessMethod === 'backup' && ( + + ) } - + - +
diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/error-message.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/error-message.tsx index 4c083992a0582..43b0d154e87f4 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/error-message.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/error-message.tsx @@ -1,15 +1,13 @@ import React from 'react'; interface Props { - error?: { - message?: string; - }; + error?: string; } export const ErrorMessage: React.FC< Props > = ( { error } ) => { - if ( ! error || ! error.message ) { + if ( ! error ) { return null; } - return
{ error.message }
; + return
{ error }
; }; diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/password-field.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/password-field.tsx index 96b6865cada7b..6decb3d6c7d6e 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/password-field.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/password-field.tsx @@ -11,10 +11,10 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - errors: any; + error?: string; } -export const PasswordField: React.FC< Props > = ( { control, errors } ) => { +export const PasswordField: React.FC< Props > = ( { control, error } ) => { const translate = useTranslate(); const [ passwordHidden, setPasswordHidden ] = useState( true ); const toggleVisibilityClasses = clsx( { @@ -38,7 +38,7 @@ export const PasswordField: React.FC< Props > = ( { control, errors } ) => { autoComplete="off" id="site-migration-credentials__password" type={ passwordHidden ? 'password' : 'text' } - isError={ !! errors.password } + isError={ !! error } placeholder={ translate( 'Enter your Admin password' ) } { ...field } /> @@ -52,7 +52,7 @@ export const PasswordField: React.FC< Props > = ( { control, errors } ) => {
) } /> - + ); }; diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/site-address-field.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/site-address-field.tsx index 89dbb155984ec..23b5f900da55f 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/site-address-field.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/site-address-field.tsx @@ -10,15 +10,11 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - errors: any; + error?: string; importSiteQueryParam?: string | undefined | null; } -export const SiteAddressField: React.FC< Props > = ( { - control, - errors, - importSiteQueryParam, -} ) => { +export const SiteAddressField: React.FC< Props > = ( { control, error, importSiteQueryParam } ) => { const translate = useTranslate(); const hasEnTranslation = useHasEnTranslation(); @@ -46,7 +42,7 @@ export const SiteAddressField: React.FC< Props > = ( { render={ ( { field } ) => ( = ( { /> ) } /> - + ); }; diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/special-instructions.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/special-instructions.tsx index da3e9d8175130..0cd371d45ba28 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/special-instructions.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/special-instructions.tsx @@ -11,10 +11,10 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - errors: any; + error?: string; } -export const SpecialInstructions: React.FC< Props > = ( { control, errors } ) => { +export const SpecialInstructions: React.FC< Props > = ( { control, error } ) => { const translate = useTranslate(); const hasEnTranslation = useHasEnTranslation(); const [ showNotes, setShowNotes ] = useState( false ); @@ -61,7 +61,7 @@ export const SpecialInstructions: React.FC< Props > = ( { control, errors } ) => ) } /> - +
{ translate( "Please don't share any passwords or secure information in this field. We'll reach out to collect that information if you have any additional credentials to access your site." diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/username-field.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/username-field.tsx index 4e506c2ed361e..f0ac7c26c83e8 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/username-field.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/username-field.tsx @@ -7,10 +7,10 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - errors: any; + error?: string; } -export const UsernameField: React.FC< Props > = ( { control, errors } ) => { +export const UsernameField: React.FC< Props > = ( { control, error } ) => { const translate = useTranslate(); return ( @@ -26,7 +26,7 @@ export const UsernameField: React.FC< Props > = ( { control, errors } ) => { ) => { @@ -40,7 +40,7 @@ export const UsernameField: React.FC< Props > = ( { control, errors } ) => { /> ) } /> - +
); }; diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-credentials-form.ts b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-credentials-form.ts index a4f08b4281c6f..c1e0b49986c97 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-credentials-form.ts +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-credentials-form.ts @@ -1,5 +1,5 @@ import { useTranslate } from 'i18n-calypso'; -import { useEffect } from 'react'; +import { useCallback, useEffect, useMemo } from 'react'; import { useForm } from 'react-hook-form'; import { useQuery } from 'calypso/landing/stepper/hooks/use-query'; import { recordTracksEvent } from 'calypso/lib/analytics/tracks'; @@ -21,73 +21,12 @@ export const useCredentialsForm = ( onSubmit: () => void ) => { const translate = useTranslate(); const importSiteQueryParam = useQuery().get( 'from' ) || ''; - const fieldMapping = { - from_url: { - fieldName: 'siteAddress', - errorMessage: translate( 'Enter a valid URL.' ), - }, - username: { - fieldName: 'username', - errorMessage: translate( 'Enter a valid username.' ), - }, - password: { - fieldName: 'password', - errorMessage: translate( 'Enter a valid password.' ), - }, - migration_type: { - fieldName: 'howToAccessSite', - errorMessage: null, - }, - notes: { - fieldName: 'notes', - errorMessage: null, - }, - }; - - const setGlobalError = ( message?: string | null | undefined ) => { - // eslint-disable-next-line @typescript-eslint/no-use-before-define - setError( 'root', { - type: 'manual', - message: message ?? translate( 'An error occurred while saving credentials.' ), - } ); - }; - - const handleMigrationError = ( err: MigrationError ) => { - let hasUnmappedFieldError = false; - - if ( err.body?.code === 'rest_invalid_param' && err.body?.data?.params ) { - Object.entries( err.body.data.params ).forEach( ( [ key ] ) => { - const field = fieldMapping[ key as keyof typeof fieldMapping ]; - const keyName = - // eslint-disable-next-line @typescript-eslint/no-use-before-define - 'backup' === accessMethod && field?.fieldName === 'siteAddress' - ? 'backupFileLocation' - : field?.fieldName; - - if ( keyName ) { - const message = field?.errorMessage ?? translate( 'Invalid input, please check again' ); - // eslint-disable-next-line @typescript-eslint/no-use-before-define - setError( keyName as keyof CredentialsFormData, { type: 'manual', message } ); - } else if ( ! hasUnmappedFieldError ) { - hasUnmappedFieldError = true; - setGlobalError(); - } - } ); - } else { - setGlobalError( err.body?.message ); - } - }; - - const { isPending, requestAutomatedMigration } = useSiteMigrationCredentialsMutation( { - onSuccess: () => { - recordTracksEvent( 'calypso_site_migration_automated_request_success' ); - onSubmit(); - }, - onError: ( error: any ) => { - handleMigrationError( mapApiError( error ) ); - recordTracksEvent( 'calypso_site_migration_automated_request_error' ); - }, - } ); + const { + isPending, + mutate: requestAutomatedMigration, + error, + isSuccess, + } = useSiteMigrationCredentialsMutation(); const { formState: { errors }, @@ -109,6 +48,85 @@ export const useCredentialsForm = ( onSubmit: () => void ) => { howToAccessSite: 'credentials', }, } ); + const accessMethod = watch( 'howToAccessSite' ); + + const fieldMapping: Record< string, { fieldName: string; errorMessage: string | null } > = + useMemo( + () => ( { + from_url: { + fieldName: 'siteAddress', + errorMessage: translate( 'Enter a valid URL.' ), + }, + username: { + fieldName: 'username', + errorMessage: translate( 'Enter a valid username.' ), + }, + password: { + fieldName: 'password', + errorMessage: translate( 'Enter a valid password.' ), + }, + migration_type: { + fieldName: 'howToAccessSite', + errorMessage: null, + }, + notes: { + fieldName: 'notes', + errorMessage: null, + }, + } ), + [ translate ] + ); + + const setGlobalError = useCallback( + ( message?: string | null ) => { + setError( 'root', { + type: 'manual', + message: message ?? translate( 'An error occurred while saving credentials.' ), + } ); + }, + [ setError, translate ] + ); + + const handleMigrationError = useCallback( + ( err: MigrationError ) => { + let hasUnmappedFieldError = false; + + if ( err.body?.code === 'rest_invalid_param' && err.body?.data?.params ) { + Object.entries( err.body.data.params ).forEach( ( [ key ] ) => { + const field = fieldMapping[ key as keyof typeof fieldMapping ]; + const keyName = + 'backup' === accessMethod && field?.fieldName === 'siteAddress' + ? 'backupFileLocation' + : field?.fieldName; + + if ( keyName ) { + const message = field?.errorMessage ?? translate( 'Invalid input, please check again' ); + setError( keyName as keyof CredentialsFormData, { type: 'manual', message } ); + } else if ( ! hasUnmappedFieldError ) { + hasUnmappedFieldError = true; + setGlobalError(); + } + } ); + } else { + setGlobalError( err.body?.message ); + } + }, + [ accessMethod, fieldMapping, setError, setGlobalError, translate ] + ); + + useEffect( () => { + if ( isSuccess ) { + recordTracksEvent( 'calypso_site_migration_automated_request_success' ); + onSubmit(); + } + }, [ isSuccess, onSubmit ] ); + + useEffect( () => { + if ( error ) { + handleMigrationError( mapApiError( error ) ); + recordTracksEvent( 'calypso_site_migration_automated_request_error' ); + } + }, [ error, handleMigrationError ] ); useEffect( () => { const { unsubscribe } = watch( () => { @@ -117,8 +135,6 @@ export const useCredentialsForm = ( onSubmit: () => void ) => { return () => unsubscribe(); }, [ watch, clearErrors ] ); - const accessMethod = watch( 'howToAccessSite' ); - const submitHandler = ( data: CredentialsFormData ) => { requestAutomatedMigration( data ); }; diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-site-migration-credentials-mutation.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-site-migration-credentials-mutation.tsx index 3ce58a8489c4b..cb2952a595501 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-site-migration-credentials-mutation.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-site-migration-credentials-mutation.tsx @@ -26,7 +26,7 @@ export const useSiteMigrationCredentialsMutation = < ) => { const siteSlug = useSiteSlugParam(); - const { mutate, ...rest } = useMutation( { + return useMutation( { mutationFn: ( { siteAddress, username, @@ -66,9 +66,4 @@ export const useSiteMigrationCredentialsMutation = < }, ...options, } ); - - return { - requestAutomatedMigration: mutate, - ...rest, - }; }; From d1a7d52acea7f6a7bfcaa160a47875d5a5f14ba3 Mon Sep 17 00:00:00 2001 From: Gabriel Caires Date: Wed, 18 Sep 2024 19:57:16 +0100 Subject: [PATCH 05/15] Refactor form --- .../components/access-method-picker.tsx | 4 +- .../components/credentials-form.tsx | 2 +- .../components/site-address-field.tsx | 6 +- .../hooks/use-form-error-mapping.ts | 63 ++++++++++++ .../site-migration-credentials/types.ts | 22 +++-- .../use-credentials-form.ts | 95 +++---------------- ...se-site-migration-credentials-mutation.tsx | 26 ++--- 7 files changed, 102 insertions(+), 116 deletions(-) create mode 100644 client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/hooks/use-form-error-mapping.ts diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/access-method-picker.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/access-method-picker.tsx index deb7ceefae481..88dc969698faa 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/access-method-picker.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/access-method-picker.tsx @@ -18,7 +18,7 @@ export const AccessMethodPicker: FC< Props > = ( { control } ) => {
( = ( { control } ) => {
( = ( { onSubmit, onSkip
diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/site-address-field.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/site-address-field.tsx index 23b5f900da55f..1b5eccf20b99c 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/site-address-field.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/site-address-field.tsx @@ -31,17 +31,17 @@ export const SiteAddressField: React.FC< Props > = ( { control, error, importSit return (
- { translate( 'Site address' ) } + { translate( 'Site address' ) } ( | 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 = ( key: string ) => { + return ( + fieldMapping[ key ] ?? { + type: 'manual', + message: translate( 'Invalid input, please check again' ), + } + ); + }; + + // 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; + }; + + const handleServerError = ( error: ApiError, { migrationType }: CredentialsFormData ) => { + const { code, message, data } = error; + 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 } > + ); + }; + + if ( error && variables ) { + return handleServerError( error, variables ) as FieldErrors< CredentialsFormData >; + } + + return undefined; +}; diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/types.ts b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/types.ts index 3180ef7eee08a..3b5231a0e8d7e 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/types.ts +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/types.ts @@ -1,4 +1,12 @@ 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; @@ -7,14 +15,10 @@ export interface CredentialsFormData { howToAccessSite: 'credentials' | 'backup'; } -export interface MigrationError { - body: { - code: string; - message: string; - data: { - status: number; - params?: Record< string, string >; - }; +export interface ApiError { + code: string; + message: string; + data: { + params?: Record< string, string >; }; - status: number; } diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-credentials-form.ts b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-credentials-form.ts index c1e0b49986c97..fb8d945269737 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-credentials-form.ts +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-credentials-form.ts @@ -1,33 +1,23 @@ -import { useTranslate } from 'i18n-calypso'; -import { useCallback, useEffect, useMemo } from 'react'; +import { useEffect } from 'react'; import { useForm } from 'react-hook-form'; import { useQuery } from 'calypso/landing/stepper/hooks/use-query'; import { recordTracksEvent } from 'calypso/lib/analytics/tracks'; -import { MigrationError, CredentialsFormData } from './types'; +import { useFormErrorMapping } from './hooks/use-form-error-mapping'; +import { CredentialsFormData } from './types'; import { useSiteMigrationCredentialsMutation } from './use-site-migration-credentials-mutation'; -const mapApiError = ( error: any ) => { - return { - body: { - code: error.code, - message: error.message, - data: error.data, - }, - status: error.status, - }; -}; - export const useCredentialsForm = ( onSubmit: () => void ) => { - const translate = useTranslate(); const importSiteQueryParam = useQuery().get( 'from' ) || ''; - const { isPending, mutate: requestAutomatedMigration, error, isSuccess, + variables, } = useSiteMigrationCredentialsMutation(); + const serverSideError = useFormErrorMapping( error, variables ); + const { formState: { errors }, control, @@ -40,79 +30,17 @@ export const useCredentialsForm = ( onSubmit: () => void ) => { reValidateMode: 'onSubmit', disabled: isPending, defaultValues: { - siteAddress: importSiteQueryParam, + from_url: importSiteQueryParam, username: '', password: '', backupFileLocation: '', notes: '', - howToAccessSite: 'credentials', + migrationType: 'credentials', }, + errors: serverSideError, } ); - const accessMethod = watch( 'howToAccessSite' ); - const fieldMapping: Record< string, { fieldName: string; errorMessage: string | null } > = - useMemo( - () => ( { - from_url: { - fieldName: 'siteAddress', - errorMessage: translate( 'Enter a valid URL.' ), - }, - username: { - fieldName: 'username', - errorMessage: translate( 'Enter a valid username.' ), - }, - password: { - fieldName: 'password', - errorMessage: translate( 'Enter a valid password.' ), - }, - migration_type: { - fieldName: 'howToAccessSite', - errorMessage: null, - }, - notes: { - fieldName: 'notes', - errorMessage: null, - }, - } ), - [ translate ] - ); - - const setGlobalError = useCallback( - ( message?: string | null ) => { - setError( 'root', { - type: 'manual', - message: message ?? translate( 'An error occurred while saving credentials.' ), - } ); - }, - [ setError, translate ] - ); - - const handleMigrationError = useCallback( - ( err: MigrationError ) => { - let hasUnmappedFieldError = false; - - if ( err.body?.code === 'rest_invalid_param' && err.body?.data?.params ) { - Object.entries( err.body.data.params ).forEach( ( [ key ] ) => { - const field = fieldMapping[ key as keyof typeof fieldMapping ]; - const keyName = - 'backup' === accessMethod && field?.fieldName === 'siteAddress' - ? 'backupFileLocation' - : field?.fieldName; - - if ( keyName ) { - const message = field?.errorMessage ?? translate( 'Invalid input, please check again' ); - setError( keyName as keyof CredentialsFormData, { type: 'manual', message } ); - } else if ( ! hasUnmappedFieldError ) { - hasUnmappedFieldError = true; - setGlobalError(); - } - } ); - } else { - setGlobalError( err.body?.message ); - } - }, - [ accessMethod, fieldMapping, setError, setGlobalError, translate ] - ); + const accessMethod = watch( 'migrationType' ); useEffect( () => { if ( isSuccess ) { @@ -123,10 +51,9 @@ export const useCredentialsForm = ( onSubmit: () => void ) => { useEffect( () => { if ( error ) { - handleMigrationError( mapApiError( error ) ); recordTracksEvent( 'calypso_site_migration_automated_request_error' ); } - }, [ error, handleMigrationError ] ); + }, [ error ] ); useEffect( () => { const { unsubscribe } = watch( () => { diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-site-migration-credentials-mutation.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-site-migration-credentials-mutation.tsx index cb2952a595501..c151b675f5bcc 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-site-migration-credentials-mutation.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-site-migration-credentials-mutation.tsx @@ -1,7 +1,7 @@ import { useMutation, UseMutationOptions } from '@tanstack/react-query'; import wpcomRequest from 'wpcom-proxy-request'; import { useSiteSlugParam } from 'calypso/landing/stepper/hooks/use-site-slug-param'; -import { CredentialsFormData } from './types'; +import { ApiError, CredentialsFormData } from './types'; interface AutomatedMigrationAPIResponse { success: boolean; @@ -18,33 +18,25 @@ interface AutomatedMigrationBody { } export const useSiteMigrationCredentialsMutation = < - TData = AutomatedMigrationAPIResponse | unknown, - TError = unknown, - TContext = unknown, + TData = AutomatedMigrationAPIResponse, + TError = ApiError, >( - options: UseMutationOptions< TData, TError, CredentialsFormData, TContext > = {} + options: UseMutationOptions< TData, TError, FormData > = {} ) => { const siteSlug = useSiteSlugParam(); - return useMutation( { - mutationFn: ( { - siteAddress, - username, - password, - notes, - howToAccessSite, - backupFileLocation, - } ) => { + return useMutation< TData, TError, CredentialsFormData >( { + mutationFn: ( { from_url, username, password, notes, migrationType, backupFileLocation } ) => { let body: AutomatedMigrationBody = { - migration_type: howToAccessSite, + migration_type: migrationType, blog_url: siteSlug ?? '', notes, }; - if ( howToAccessSite === 'credentials' ) { + if ( migrationType === 'credentials' ) { body = { ...body, - from_url: siteAddress, + from_url, username, password, }; From 7f9e9d6fbcb0be98818f86697f5e53c7943d6a1f Mon Sep 17 00:00:00 2001 From: Gabriel Caires Date: Wed, 18 Sep 2024 20:07:27 +0100 Subject: [PATCH 06/15] Remove errrors changes --- .../components/backup-file-field.tsx | 10 +++++----- .../components/credentials-form.tsx | 14 ++++++-------- .../components/error-message.tsx | 8 +++++--- .../components/password-field.tsx | 8 ++++---- .../components/site-address-field.tsx | 12 ++++++++---- .../components/special-instructions.tsx | 6 +++--- .../components/username-field.tsx | 8 ++++---- 7 files changed, 35 insertions(+), 31 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx index 14de5c706fb63..8a7996b2c2408 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx @@ -1,6 +1,6 @@ import { FormLabel } from '@automattic/components'; import { useTranslate } from 'i18n-calypso'; -import { Controller, Control } from 'react-hook-form'; +import { Controller, Control, FieldErrors } from 'react-hook-form'; import FormTextInput from 'calypso/components/forms/form-text-input'; import { isValidUrl } from 'calypso/lib/importer/url-validation'; import { CredentialsFormData } from '../types'; @@ -8,10 +8,10 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - error?: string; + errors?: FieldErrors< CredentialsFormData >; } -export const BackupFileField: React.FC< Props > = ( { control, error } ) => { +export const BackupFileField: React.FC< Props > = ( { control, errors } ) => { const translate = useTranslate(); const isBackupFileLocationValid = ( fileLocation: string ) => { @@ -34,14 +34,14 @@ export const BackupFileField: React.FC< Props > = ( { control, error } ) => { ) } />
- +
{ translate( "Upload your file to a service like Dropbox or Google Drive to get a link. Don't forget to make sure that anyone with the link can access it." diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/credentials-form.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/credentials-form.tsx index 58f047ddd429f..08eab372d557b 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/credentials-form.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/credentials-form.tsx @@ -54,22 +54,20 @@ export const CredentialsForm: FC< CredentialsFormProps > = ( { onSubmit, onSkip
- - + +
) } - { accessMethod === 'backup' && ( - - ) } + { accessMethod === 'backup' && } - + - +
diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/error-message.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/error-message.tsx index 43b0d154e87f4..4c083992a0582 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/error-message.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/error-message.tsx @@ -1,13 +1,15 @@ import React from 'react'; interface Props { - error?: string; + error?: { + message?: string; + }; } export const ErrorMessage: React.FC< Props > = ( { error } ) => { - if ( ! error ) { + if ( ! error || ! error.message ) { return null; } - return
{ error }
; + return
{ error.message }
; }; diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/password-field.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/password-field.tsx index 6decb3d6c7d6e..96b6865cada7b 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/password-field.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/password-field.tsx @@ -11,10 +11,10 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - error?: string; + errors: any; } -export const PasswordField: React.FC< Props > = ( { control, error } ) => { +export const PasswordField: React.FC< Props > = ( { control, errors } ) => { const translate = useTranslate(); const [ passwordHidden, setPasswordHidden ] = useState( true ); const toggleVisibilityClasses = clsx( { @@ -38,7 +38,7 @@ export const PasswordField: React.FC< Props > = ( { control, error } ) => { autoComplete="off" id="site-migration-credentials__password" type={ passwordHidden ? 'password' : 'text' } - isError={ !! error } + isError={ !! errors.password } placeholder={ translate( 'Enter your Admin password' ) } { ...field } /> @@ -52,7 +52,7 @@ export const PasswordField: React.FC< Props > = ( { control, error } ) => {
) } /> - +
); }; diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/site-address-field.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/site-address-field.tsx index 1b5eccf20b99c..c2fafec36a270 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/site-address-field.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/site-address-field.tsx @@ -10,11 +10,15 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - error?: string; + errors?: any; importSiteQueryParam?: string | undefined | null; } -export const SiteAddressField: React.FC< Props > = ( { control, error, importSiteQueryParam } ) => { +export const SiteAddressField: React.FC< Props > = ( { + control, + errors, + importSiteQueryParam, +} ) => { const translate = useTranslate(); const hasEnTranslation = useHasEnTranslation(); @@ -42,7 +46,7 @@ export const SiteAddressField: React.FC< Props > = ( { control, error, importSit render={ ( { field } ) => ( = ( { control, error, importSit /> ) } /> - +
); }; diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/special-instructions.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/special-instructions.tsx index 0cd371d45ba28..be1762d5c3b9d 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/special-instructions.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/special-instructions.tsx @@ -11,10 +11,10 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - error?: string; + errors?: any; } -export const SpecialInstructions: React.FC< Props > = ( { control, error } ) => { +export const SpecialInstructions: React.FC< Props > = ( { control, errors } ) => { const translate = useTranslate(); const hasEnTranslation = useHasEnTranslation(); const [ showNotes, setShowNotes ] = useState( false ); @@ -61,7 +61,7 @@ export const SpecialInstructions: React.FC< Props > = ( { control, error } ) => ) } />
- +
{ translate( "Please don't share any passwords or secure information in this field. We'll reach out to collect that information if you have any additional credentials to access your site." diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/username-field.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/username-field.tsx index f0ac7c26c83e8..8394ba56fd673 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/username-field.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/username-field.tsx @@ -7,10 +7,10 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - error?: string; + errors?: any; } -export const UsernameField: React.FC< Props > = ( { control, error } ) => { +export const UsernameField: React.FC< Props > = ( { control, errors } ) => { const translate = useTranslate(); return ( @@ -26,7 +26,7 @@ export const UsernameField: React.FC< Props > = ( { control, error } ) => { ) => { @@ -40,7 +40,7 @@ export const UsernameField: React.FC< Props > = ( { control, error } ) => { /> ) } /> - +
); }; From b241fb4a944062b75ecf7626517447df7bc8f16c Mon Sep 17 00:00:00 2001 From: Gabriel Caires Date: Wed, 18 Sep 2024 20:09:24 +0100 Subject: [PATCH 07/15] Remove errrors changes --- .../components/backup-file-field.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx index 8a7996b2c2408..f42825f7782d1 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx @@ -1,6 +1,6 @@ import { FormLabel } from '@automattic/components'; import { useTranslate } from 'i18n-calypso'; -import { Controller, Control, FieldErrors } from 'react-hook-form'; +import { Controller, Control } from 'react-hook-form'; import FormTextInput from 'calypso/components/forms/form-text-input'; import { isValidUrl } from 'calypso/lib/importer/url-validation'; import { CredentialsFormData } from '../types'; @@ -8,7 +8,7 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - errors?: FieldErrors< CredentialsFormData >; + errors?: any; } export const BackupFileField: React.FC< Props > = ( { control, errors } ) => { @@ -34,14 +34,14 @@ export const BackupFileField: React.FC< Props > = ( { control, errors } ) => { ) } /> - +
{ translate( "Upload your file to a service like Dropbox or Google Drive to get a link. Don't forget to make sure that anyone with the link can access it." From def978dc21de2fa0ffef2165ea9a4b1057c8ee28 Mon Sep 17 00:00:00 2001 From: Gabriel Caires Date: Thu, 19 Sep 2024 10:08:56 +0100 Subject: [PATCH 08/15] Reduce screen reference repetition --- .../site-migration-credentials/test/index.tsx | 64 +++++++++---------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx index 0d633d4d88619..474f6d596f082 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx @@ -29,19 +29,19 @@ const messages = { "Looks like your site address is missing its domain extension. Please try again with something like 'example.com' or 'example.net'.", }; -const continueButton = () => screen.getByRole( 'button', { name: /Continue/ } ); -const siteAddressInput = () => screen.getByLabelText( 'Site address' ); -const usernameInput = () => screen.getByLabelText( 'WordPress admin username' ); -const passwordInput = () => screen.getByLabelText( 'Password' ); -const backupOption = () => screen.getByRole( 'radio', { name: 'Backup file' } ); -const credentialsOption = () => screen.getByRole( 'radio', { name: 'WordPress credentials' } ); -const backupFileInput = () => screen.getByLabelText( 'Backup file location' ); +const { getByRole, getByLabelText, getByTestId, getByText, findByText } = screen; + +const continueButton = () => getByRole( 'button', { name: /Continue/ } ); +const siteAddressInput = () => getByLabelText( 'Site address' ); +const usernameInput = () => getByLabelText( 'WordPress admin username' ); +const passwordInput = () => getByLabelText( 'Password' ); +const backupOption = () => getByRole( 'radio', { name: 'Backup file' } ); +const credentialsOption = () => getByRole( 'radio', { name: 'WordPress credentials' } ); +const backupFileInput = () => getByLabelText( 'Backup file location' ); //TODO: it requires a testid because there is no accessible name, it is an issue with the component -const specialInstructionsInput = () => screen.getByTestId( 'special-instructions-textarea' ); -const specialInstructionsButton = () => - screen.getByRole( 'button', { name: 'Special instructions' } ); -const skipButton = () => - screen.getByRole( 'button', { name: /Skip, I need help providing access/ } ); +const specialInstructionsInput = () => getByTestId( 'special-instructions-textarea' ); +const specialInstructionsButton = () => getByRole( 'button', { name: 'Special instructions' } ); +const skipButton = () => getByRole( 'button', { name: /Skip, I need help providing access/ } ); const fillAllFields = async () => { await userEvent.click( credentialsOption() ); @@ -89,16 +89,6 @@ describe( 'SiteMigrationCredentials', () => { } ); } ); - it( 'skips the credential creation when the user does not fill the fields', async () => { - const submit = jest.fn(); - render( { navigation: { submit } } ); - - await userEvent.click( skipButton() ); - - expect( submit ).toHaveBeenCalledWith( { action: 'skip' } ); - expect( wpcomRequest ).not.toHaveBeenCalled(); - } ); - it( 'creates a credentials using backup file', async () => { const submit = jest.fn(); render( { navigation: { submit } } ); @@ -128,13 +118,23 @@ describe( 'SiteMigrationCredentials', () => { } ); } ); + it( 'skips the credential creation when the user does not fill the fields', async () => { + const submit = jest.fn(); + render( { navigation: { submit } } ); + + await userEvent.click( skipButton() ); + + expect( submit ).toHaveBeenCalledWith( { action: 'skip' } ); + expect( wpcomRequest ).not.toHaveBeenCalled(); + } ); + it( 'shows errors on the required fields when the user does not fill the fields', async () => { render(); await userEvent.click( continueButton() ); - expect( screen.getByText( messages.urlError ) ).toBeInTheDocument(); - expect( screen.getByText( messages.usernameError ) ).toBeInTheDocument(); - expect( screen.getByText( messages.passwordError ) ).toBeInTheDocument(); + expect( getByText( messages.urlError ) ).toBeInTheDocument(); + expect( getByText( messages.usernameError ) ).toBeInTheDocument(); + expect( getByText( messages.passwordError ) ).toBeInTheDocument(); } ); it( 'shows error when user set invalid site address', async () => { @@ -142,7 +142,7 @@ describe( 'SiteMigrationCredentials', () => { await userEvent.type( siteAddressInput(), 'invalid-site-address' ); await userEvent.click( continueButton() ); - expect( screen.getByText( messages.noTLDError ) ).toBeInTheDocument(); + expect( getByText( messages.noTLDError ) ).toBeInTheDocument(); } ); it( 'fills the site address and disable it when the user already informed the site address on previous step', async () => { @@ -172,9 +172,9 @@ describe( 'SiteMigrationCredentials', () => { await fillAllFields(); await userEvent.click( continueButton() ); await waitFor( () => { - expect( screen.getByText( /Enter a valid URL/ ) ).toBeVisible(); - expect( screen.getByText( /Enter a valid username/ ) ).toBeVisible(); - expect( screen.getByText( /Enter a valid password/ ) ).toBeVisible(); + expect( getByText( /Enter a valid URL/ ) ).toBeVisible(); + expect( getByText( /Enter a valid username/ ) ).toBeVisible(); + expect( getByText( /Enter a valid password/ ) ).toBeVisible(); expect( submit ).not.toHaveBeenCalled(); } ); } ); @@ -197,7 +197,7 @@ describe( 'SiteMigrationCredentials', () => { await userEvent.click( continueButton() ); await waitFor( () => { - expect( screen.getByText( /Enter a valid URL/ ) ).toBeVisible(); + expect( getByText( /Enter a valid URL/ ) ).toBeVisible(); expect( submit ).not.toHaveBeenCalled(); } ); } ); @@ -214,7 +214,7 @@ describe( 'SiteMigrationCredentials', () => { await fillAllFields(); await userEvent.click( continueButton() ); - expect( screen.getByText( /Error message from backend/ ) ).toBeVisible(); + expect( getByText( /Error message from backend/ ) ).toBeVisible(); expect( submit ).not.toHaveBeenCalled(); } ); @@ -224,7 +224,7 @@ describe( 'SiteMigrationCredentials', () => { render( { navigation: { submit } }, { initialEntry } ); - const errorMessage = await screen.findByText( + const errorMessage = await findByText( /We ran into a problem submitting your details. Please try again shortly./ ); expect( errorMessage ).toBeVisible(); From 203221af5dc4aa9517422f4beb3ad80c1c727814 Mon Sep 17 00:00:00 2001 From: Gabriel Caires Date: Thu, 19 Sep 2024 11:25:46 +0100 Subject: [PATCH 09/15] Remove uncessary changes --- .../site-migration-credentials/components/backup-file-field.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx index f42825f7782d1..a055a9bcc7b97 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/backup-file-field.tsx @@ -8,7 +8,7 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - errors?: any; + errors: any; } export const BackupFileField: React.FC< Props > = ( { control, errors } ) => { From d62b7a10a727a812fc49f54cad8b039e3abf96a2 Mon Sep 17 00:00:00 2001 From: Gabriel Caires Date: Thu, 19 Sep 2024 11:28:08 +0100 Subject: [PATCH 10/15] Remove uncessary changes --- .../components/special-instructions.tsx | 2 +- .../site-migration-credentials/components/username-field.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/special-instructions.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/special-instructions.tsx index be1762d5c3b9d..da3e9d8175130 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/special-instructions.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/special-instructions.tsx @@ -11,7 +11,7 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - errors?: any; + errors: any; } export const SpecialInstructions: React.FC< Props > = ( { control, errors } ) => { diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/username-field.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/username-field.tsx index 8394ba56fd673..4e506c2ed361e 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/username-field.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/components/username-field.tsx @@ -7,7 +7,7 @@ import { ErrorMessage } from './error-message'; interface Props { control: Control< CredentialsFormData >; - errors?: any; + errors: any; } export const UsernameField: React.FC< Props > = ( { control, errors } ) => { From c458551cbabe1ae864d2081ed34b097fe46d6426 Mon Sep 17 00:00:00 2001 From: Gabriel Caires Date: Thu, 19 Sep 2024 12:14:43 +0100 Subject: [PATCH 11/15] Handle more error cases --- .../hooks/use-form-error-mapping.ts | 90 +++++++++++-------- .../site-migration-credentials/test/index.tsx | 12 +++ 2 files changed, 66 insertions(+), 36 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/hooks/use-form-error-mapping.ts b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/hooks/use-form-error-mapping.ts index 4bdfa6ac8ec54..e47e66d98edec 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/hooks/use-form-error-mapping.ts +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/hooks/use-form-error-mapping.ts @@ -1,8 +1,15 @@ import { useTranslate } from 'i18n-calypso'; -import { useMemo } from 'react'; +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, @@ -20,44 +27,55 @@ export const useFormErrorMapping = ( [ translate ] ); - const getTranslatedMessage = ( key: string ) => { - return ( - fieldMapping[ key ] ?? { - type: 'manual', - message: translate( 'Invalid input, please check again' ), + 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.' ), + }, + }; } - ); - }; - - // 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; - }; - - const handleServerError = ( error: ApiError, { migrationType }: CredentialsFormData ) => { - const { code, message, data } = error; - 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 ); + if ( code !== 'rest_invalid_param' || ! data?.params ) { + return { root: { type: 'manual', message } }; + } - errors[ fieldName ] = message; - return errors; - }, - {} as Record< string, { type: string; message: string } > - ); - }; + const invalidFields = Object.keys( data.params ); - if ( error && variables ) { - return handleServerError( error, variables ) as FieldErrors< CredentialsFormData >; - } + return invalidFields.reduce( + ( errors, key ) => { + const fieldName = getFieldName( key, migrationType ); + const message = getTranslatedMessage( key ); - return undefined; + 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 ] ); }; diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx index 474f6d596f082..3dccd000c31bc 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx @@ -218,6 +218,18 @@ describe( 'SiteMigrationCredentials', () => { 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() ); + + 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'; From 82923614899b13bdba8ae9bfe109de03c20639a6 Mon Sep 17 00:00:00 2001 From: Gabriel Caires Date: Thu, 19 Sep 2024 12:30:59 +0100 Subject: [PATCH 12/15] Add more test scenarios --- .../site-migration-credentials/test/index.tsx | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx index 3dccd000c31bc..5d7b651e2f6d2 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx @@ -128,13 +128,20 @@ 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 () => { @@ -142,7 +149,7 @@ describe( 'SiteMigrationCredentials', () => { 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 () => { From 6795cbb83e99abb2bae7a6b09d2ebb308c477bfa Mon Sep 17 00:00:00 2001 From: Gabriel Caires Date: Thu, 19 Sep 2024 12:53:11 +0100 Subject: [PATCH 13/15] Fix type --- .../use-credentials-form.ts | 2 -- .../use-site-migration-credentials-mutation.tsx | 11 +++++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-credentials-form.ts b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-credentials-form.ts index fb8d945269737..c1eb87c47a0db 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-credentials-form.ts +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-credentials-form.ts @@ -23,7 +23,6 @@ export const useCredentialsForm = ( onSubmit: () => void ) => { control, handleSubmit, watch, - setError, clearErrors, } = useForm< CredentialsFormData >( { mode: 'onSubmit', @@ -74,7 +73,6 @@ export const useCredentialsForm = ( onSubmit: () => void ) => { accessMethod, isPending, submitHandler, - setError, importSiteQueryParam, }; }; diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-site-migration-credentials-mutation.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-site-migration-credentials-mutation.tsx index c151b675f5bcc..5fe17bc455d8b 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-site-migration-credentials-mutation.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/use-site-migration-credentials-mutation.tsx @@ -21,12 +21,19 @@ export const useSiteMigrationCredentialsMutation = < TData = AutomatedMigrationAPIResponse, TError = ApiError, >( - options: UseMutationOptions< TData, TError, FormData > = {} + options: UseMutationOptions< TData, TError, CredentialsFormData > = {} ) => { const siteSlug = useSiteSlugParam(); return useMutation< TData, TError, CredentialsFormData >( { - mutationFn: ( { from_url, username, password, notes, migrationType, backupFileLocation } ) => { + mutationFn: ( { + from_url, + username, + password, + notes, + migrationType, + backupFileLocation, + }: CredentialsFormData ) => { let body: AutomatedMigrationBody = { migration_type: migrationType, blog_url: siteSlug ?? '', From bdd0b2475fcdb05b35c0d6cd6b3ce8a8b9b8db87 Mon Sep 17 00:00:00 2001 From: Imran Hossain Date: Sat, 21 Sep 2024 01:25:33 +0600 Subject: [PATCH 14/15] Fix linting --- .../steps-repository/site-migration-credentials/types.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/types.ts b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/types.ts index d31fcce5a9110..a3f32c8ec54e5 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/types.ts +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/types.ts @@ -23,7 +23,6 @@ export interface ApiError { message: string; data: { params?: Record< string, string >; - }; } From 86f601377cc42f4ad26ab4d208db51a51782c61d Mon Sep 17 00:00:00 2001 From: Imran Hossain Date: Sat, 21 Sep 2024 16:27:57 +0600 Subject: [PATCH 15/15] Fix unit tests --- .../site-migration-credentials/test/index.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx index 2c849686ba38c..af24545c40104 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-credentials/test/index.tsx @@ -221,7 +221,9 @@ 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(); } ); @@ -234,7 +236,9 @@ describe( 'SiteMigrationCredentials', () => { await fillAllFields(); await userEvent.click( continueButton() ); - expect( getByText( /An error occurred while saving credentials./ ) ).toBeVisible(); + await waitFor( () => { + expect( getByText( /An error occurred while saving credentials./ ) ).toBeVisible(); + } ); } ); it( 'shows a notice when URL contains error=ticket-creation', async () => { @@ -246,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(); + } ); } ); } );