From df6a9e073e283947f7dbfae214f4be29b97e8c99 Mon Sep 17 00:00:00 2001 From: Miroslav Mitev Date: Wed, 23 Oct 2024 14:25:50 +0300 Subject: [PATCH 1/5] Migration Flow: Fix race condition when adding the blog stickers --- .../use-update-migration-status.ts | 14 +++++++-- .../site-migration-how-to-migrate/index.tsx | 29 ++++++++++++------- .../site-migration-how-to-migrate/style.scss | 5 ++++ .../use-pending-migration-status.ts | 12 +++++--- 4 files changed, 43 insertions(+), 17 deletions(-) diff --git a/client/data/site-migration/use-update-migration-status.ts b/client/data/site-migration/use-update-migration-status.ts index 3838dbf179216..cdc9aa9fc50e0 100644 --- a/client/data/site-migration/use-update-migration-status.ts +++ b/client/data/site-migration/use-update-migration-status.ts @@ -20,7 +20,11 @@ export const useUpdateMigrationStatus = () => { } ), } ); - const { mutate: updateMigrationStatusMutate, ...updateStatusMutationRest } = updateStatusMutation; + const { + mutate: updateMigrationStatusMutate, + mutateAsync: updateMigrationStatusMutateAsync, + ...updateStatusMutationRest + } = updateStatusMutation; const updateMigrationStatus = useCallback( ( targetBlogId: SiteId, statusSticker: string ) => @@ -28,5 +32,11 @@ export const useUpdateMigrationStatus = () => { [ updateMigrationStatusMutate ] ); - return { updateMigrationStatus, updateStatusMutationRest }; + const updateMigrationStatusAsync = useCallback( + ( targetBlogId: SiteId, statusSticker: string ) => + updateMigrationStatusMutateAsync( { targetBlogId, statusSticker } ), + [ updateMigrationStatusMutateAsync ] + ); + + return { updateMigrationStatus, updateMigrationStatusAsync, updateStatusMutationRest }; }; diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/index.tsx index f0ae72294280c..6269b3cf2d3cc 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/index.tsx @@ -3,6 +3,7 @@ import { useTranslate } from 'i18n-calypso'; import { FC, useMemo } from 'react'; import DocumentHead from 'calypso/components/data/document-head'; import FormattedHeader from 'calypso/components/formatted-header'; +import { LoadingEllipsis } from 'calypso/components/loading-ellipsis'; import { useAnalyzeUrlQuery } from 'calypso/data/site-profiler/use-analyze-url-query'; import { useHostingProviderQuery } from 'calypso/data/site-profiler/use-hosting-provider-query'; import { HOW_TO_MIGRATE_OPTIONS } from 'calypso/landing/stepper/constants'; @@ -62,7 +63,9 @@ const SiteMigrationHowToMigrate: FC< Props > = ( props ) => { urlData ); - const { setPendingMigration } = usePendingMigrationStatus( { onSubmit: navigation.submit } ); + const { setPendingMigration, isLoading } = usePendingMigrationStatus( { + onSubmit: navigation.submit, + } ); const hostingProviderSlug = hostingProviderData?.hosting_provider?.slug; const shouldDisplayHostIdentificationMessage = @@ -72,16 +75,20 @@ const SiteMigrationHowToMigrate: FC< Props > = ( props ) => { const stepContent = ( <> -
- { options.map( ( option, i ) => ( - setPendingMigration( option.value ) } - /> - ) ) } -
+ { isLoading ? ( + + ) : ( +
+ { options.map( ( option, i ) => ( + setPendingMigration( option.value ) } + /> + ) ) } +
+ ) } ); diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/style.scss b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/style.scss index fa615325a8a8e..7f28d87751bd9 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/style.scss +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/style.scss @@ -43,4 +43,9 @@ .how-to-migrate__description { color: var(--studio-gray-60); } + + .how-to-migrate__loader { + display: block; + margin: 0 auto; + } } diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/use-pending-migration-status.ts b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/use-pending-migration-status.ts index 8298129f1898d..4eb46178d890a 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/use-pending-migration-status.ts +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/use-pending-migration-status.ts @@ -18,7 +18,11 @@ const usePendingMigrationStatus = ( { onSubmit }: PendingMigrationStatusProps ) ? true : false; - const { updateMigrationStatus } = useUpdateMigrationStatus(); + const { + updateMigrationStatus, + updateMigrationStatusAsync, + updateStatusMutationRest: { isPending: isLoading }, + } = useUpdateMigrationStatus(); // Register pending migration status when loading the step. useEffect( () => { @@ -27,11 +31,11 @@ const usePendingMigrationStatus = ( { onSubmit }: PendingMigrationStatusProps ) } }, [ siteId, updateMigrationStatus ] ); - const setPendingMigration = ( how: string ) => { + const setPendingMigration = async ( how: string ) => { const destination = canInstallPlugins ? 'migrate' : 'upgrade'; if ( siteId ) { const parsedHow = how === HOW_TO_MIGRATE_OPTIONS.DO_IT_MYSELF ? 'diy' : how; - updateMigrationStatus( siteId, `migration-pending-${ parsedHow }` ); + await updateMigrationStatusAsync( siteId, `migration-pending-${ parsedHow }` ); } if ( onSubmit ) { @@ -39,7 +43,7 @@ const usePendingMigrationStatus = ( { onSubmit }: PendingMigrationStatusProps ) } }; - return { setPendingMigration }; + return { setPendingMigration, isLoading }; }; export default usePendingMigrationStatus; From 682f844641964d6e74f6f6064ae2b8a42d045458 Mon Sep 17 00:00:00 2001 From: Miroslav Mitev Date: Mon, 28 Oct 2024 14:58:41 +0200 Subject: [PATCH 2/5] Remove excessive fragment tag --- .../site-migration-how-to-migrate/index.tsx | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/index.tsx index 6269b3cf2d3cc..955196030de5f 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/index.tsx @@ -73,23 +73,19 @@ const SiteMigrationHowToMigrate: FC< Props > = ( props ) => { hostingProviderSlug !== 'unknown' && hostingProviderSlug !== 'automattic'; - const stepContent = ( - <> - { isLoading ? ( - - ) : ( -
- { options.map( ( option, i ) => ( - setPendingMigration( option.value ) } - /> - ) ) } -
- ) } - + const stepContent = isLoading ? ( + + ) : ( +
+ { options.map( ( option, i ) => ( + setPendingMigration( option.value ) } + /> + ) ) } +
); const platformText = shouldDisplayHostIdentificationMessage From 02527876960d205defe96bdf88c40bf63e090f13 Mon Sep 17 00:00:00 2001 From: Miroslav Mitev Date: Mon, 28 Oct 2024 16:40:21 +0200 Subject: [PATCH 3/5] Fix loading state flickering --- .../use-pending-migration-status.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/use-pending-migration-status.ts b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/use-pending-migration-status.ts index 4eb46178d890a..abf1c886ee785 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/use-pending-migration-status.ts +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/use-pending-migration-status.ts @@ -21,9 +21,14 @@ const usePendingMigrationStatus = ( { onSubmit }: PendingMigrationStatusProps ) const { updateMigrationStatus, updateMigrationStatusAsync, - updateStatusMutationRest: { isPending: isLoading }, + updateStatusMutationRest: { + isIdle: isMigrationStatusUpdateIdle, + isPending: isMigrationStatusUpdatePending, + }, } = useUpdateMigrationStatus(); + const isLoading = isMigrationStatusUpdateIdle || isMigrationStatusUpdatePending; + // Register pending migration status when loading the step. useEffect( () => { if ( siteId ) { From a9fd24e414cb89fffa385e3a1b39aae152beb74f Mon Sep 17 00:00:00 2001 From: Miroslav Mitev Date: Mon, 28 Oct 2024 16:46:31 +0200 Subject: [PATCH 4/5] Fix tests --- .../site-migration-how-to-migrate/test/index.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/test/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/test/index.tsx index 462bb5a70898c..14d26de48e7a7 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/test/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/test/index.tsx @@ -47,6 +47,8 @@ describe( 'SiteMigrationHowToMigrate', () => { mockUpdateMigrationStatus = jest.fn(); ( useUpdateMigrationStatus as jest.Mock ).mockReturnValue( { updateMigrationStatus: mockUpdateMigrationStatus, + updateMigrationStatusAsync: mockUpdateMigrationStatus, + updateStatusMutationRest: {}, } ); } ); From eff54d4a14c8ba05ac28bfab6fed2b10a16d6916 Mon Sep 17 00:00:00 2001 From: Miroslav Mitev Date: Mon, 28 Oct 2024 19:12:24 +0200 Subject: [PATCH 5/5] Fix flickering after clicking an option --- .../site-migration-how-to-migrate/index.tsx | 44 ++++++++++++------- .../test/index.tsx | 30 +++++++++---- 2 files changed, 49 insertions(+), 25 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/index.tsx index 955196030de5f..7324908b60c8e 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/index.tsx @@ -1,6 +1,6 @@ import { StepContainer } from '@automattic/onboarding'; import { useTranslate } from 'i18n-calypso'; -import { FC, useMemo } from 'react'; +import { FC, useMemo, useState } from 'react'; import DocumentHead from 'calypso/components/data/document-head'; import FormattedHeader from 'calypso/components/formatted-header'; import { LoadingEllipsis } from 'calypso/components/loading-ellipsis'; @@ -63,30 +63,42 @@ const SiteMigrationHowToMigrate: FC< Props > = ( props ) => { urlData ); - const { setPendingMigration, isLoading } = usePendingMigrationStatus( { + const { setPendingMigration, isLoading: isUpdatingMigrationStatus } = usePendingMigrationStatus( { onSubmit: navigation.submit, } ); + const [ isSubmitting, setIsSubmitting ] = useState( false ); + const handleClick = async ( value: string ) => { + setIsSubmitting( true ); + + try { + await setPendingMigration( value ); + } finally { + setIsSubmitting( false ); + } + }; + const hostingProviderSlug = hostingProviderData?.hosting_provider?.slug; const shouldDisplayHostIdentificationMessage = hostingProviderSlug && hostingProviderSlug !== 'unknown' && hostingProviderSlug !== 'automattic'; - const stepContent = isLoading ? ( - - ) : ( -
- { options.map( ( option, i ) => ( - setPendingMigration( option.value ) } - /> - ) ) } -
- ); + const stepContent = + isSubmitting || isUpdatingMigrationStatus ? ( + + ) : ( +
+ { options.map( ( option, i ) => ( + handleClick( option.value ) } + /> + ) ) } +
+ ); const platformText = shouldDisplayHostIdentificationMessage ? translate( 'Your WordPress site is hosted with %(hostingProviderName)s.', { diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/test/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/test/index.tsx index 14d26de48e7a7..cff9de56ba18a 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/test/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/site-migration-how-to-migrate/test/index.tsx @@ -1,7 +1,7 @@ /** * @jest-environment jsdom */ -import { fireEvent } from '@testing-library/react'; +import { act, fireEvent } from '@testing-library/react'; import React from 'react'; import { useUpdateMigrationStatus } from 'calypso/data/site-migration/use-update-migration-status'; import { RenderStepOptions, mockStepProps, renderStep } from '../../test/helpers'; @@ -58,11 +58,14 @@ describe( 'SiteMigrationHowToMigrate', () => { expect( mockUpdateMigrationStatus ).toHaveBeenCalledWith( siteId, 'migration-pending' ); } ); - it( 'should call updateMigrationStatus with correct value for DIFM option', () => { + it( 'should call updateMigrationStatus with correct value for DIFM option', async () => { const { getByText } = render( { navigation: { submit: mockSubmit } } ); const optionButton = getByText( 'Do it for me' ); - fireEvent.click( optionButton ); + + await act( async () => { + await fireEvent.click( optionButton ); + } ); // Check the last call value const lastCallValue = @@ -70,11 +73,14 @@ describe( 'SiteMigrationHowToMigrate', () => { expect( lastCallValue ).toBe( 'migration-pending-difm' ); } ); - it( 'should call updateMigrationStatus with correct value for DIY option', () => { + it( 'should call updateMigrationStatus with correct value for DIY option', async () => { const { getByText } = render( { navigation: { submit: mockSubmit } } ); const optionButton = getByText( "I'll do it myself" ); - fireEvent.click( optionButton ); + + await act( async () => { + await fireEvent.click( optionButton ); + } ); // Check the last call value const lastCallValue = @@ -82,20 +88,26 @@ describe( 'SiteMigrationHowToMigrate', () => { expect( lastCallValue ).toBe( 'migration-pending-diy' ); } ); - it( 'should call submit with correct value when DIFM option is clicked', () => { + it( 'should call submit with correct value when DIFM option is clicked', async () => { const { getByText } = render( { navigation: { submit: mockSubmit } } ); const optionButton = getByText( 'Do it for me' ); - fireEvent.click( optionButton ); + + await act( async () => { + await fireEvent.click( optionButton ); + } ); expect( mockSubmit ).toHaveBeenCalledWith( { destination: 'upgrade', how: 'difm' } ); } ); - it( 'should call submit with correct value for DIY option', () => { + it( 'should call submit with correct value for DIY option', async () => { const { getByText } = render( { navigation: { submit: mockSubmit } } ); const optionButton = getByText( "I'll do it myself" ); - fireEvent.click( optionButton ); + + await act( async () => { + await fireEvent.click( optionButton ); + } ); expect( mockSubmit ).toHaveBeenCalledWith( { destination: 'upgrade', how: 'myself' } ); } );