diff --git a/js/src/components/google-combo-account-card/connected-google-combo-account-card.js b/js/src/components/google-combo-account-card/connected-google-combo-account-card.js index a7f0b68a5f..bbc96c8f94 100644 --- a/js/src/components/google-combo-account-card/connected-google-combo-account-card.js +++ b/js/src/components/google-combo-account-card/connected-google-combo-account-card.js @@ -8,9 +8,9 @@ import { useEffect, useState } from '@wordpress/element'; */ import AccountCard, { APPEARANCE } from '../account-card'; import AccountDetails from './account-details'; -import AppSpinner from '../app-spinner'; import Indicator from './indicator'; import getAccountCreationTexts from './getAccountCreationTexts'; +import SpinnerCard from '.~/components/spinner-card'; import useAutoCreateAdsMCAccounts from '.~/hooks/useAutoCreateAdsMCAccounts'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import useGoogleMCAccount from '.~/hooks/useGoogleMCAccount'; @@ -54,7 +54,7 @@ const ConnectedGoogleComboAccountCard = () => { ! hasFinishedResolutionForCurrentAdsAccount || ! hasFinishedResolutionForCurrentMCAccount ) ) { - return } />; + return ; } return ( diff --git a/js/src/components/google-combo-account-card/indicator.js b/js/src/components/google-combo-account-card/indicator.js index dcbb293509..5a65c0b6b7 100644 --- a/js/src/components/google-combo-account-card/indicator.js +++ b/js/src/components/google-combo-account-card/indicator.js @@ -9,7 +9,7 @@ import { __ } from '@wordpress/i18n'; import ConnectedIconLabel from '.~/components/connected-icon-label'; import LoadingLabel from '.~/components/loading-label/loading-label'; import useGoogleAdsAccountReady from '.~/hooks/useGoogleAdsAccountReady'; -import useGoogleMCAccountReady from '.~/hooks/useGoogleMCAccountReady'; +import useGoogleMCAccount from '.~/hooks/useGoogleMCAccount'; /** * Account creation indicator. @@ -20,7 +20,7 @@ import useGoogleMCAccountReady from '.~/hooks/useGoogleMCAccountReady'; */ const Indicator = ( { showSpinner } ) => { const isGoogleAdsConnected = useGoogleAdsAccountReady(); - const isGoogleMCConnected = useGoogleMCAccountReady(); + const { isReady: isGoogleMCConnected } = useGoogleMCAccount(); if ( showSpinner ) { return ( diff --git a/js/src/hooks/useAutoCreateAdsMCAccounts.js b/js/src/hooks/useAutoCreateAdsMCAccounts.js index 6bccf39a1b..e295f263bb 100644 --- a/js/src/hooks/useAutoCreateAdsMCAccounts.js +++ b/js/src/hooks/useAutoCreateAdsMCAccounts.js @@ -58,7 +58,6 @@ const useShouldCreateMCAccount = () => { /** * @typedef {Object} AutoCreateAdsMCAccountsData - * @property {boolean} accountsCreated - Whether the accounts have been successfully created. * @property {boolean} hasDetermined - Whether the checks to determine if accounts should be created are finished. * @property {('ads'|'mc'|'both'|null)} creatingWhich - Which accounts are being created ('ads', 'mc', 'both'), or `null` if none. */ @@ -71,7 +70,6 @@ const useShouldCreateMCAccount = () => { */ const useAutoCreateAdsMCAccounts = () => { const lockedRef = useRef( false ); - // Create separate states. const [ creatingWhich, setCreatingWhich ] = useState( null ); const [ hasDetermined, setDetermined ] = useState( false ); @@ -112,14 +110,12 @@ const useAutoCreateAdsMCAccounts = () => { if ( which === CREATING_BOTH_ACCOUNTS ) { await handleCreateAccount(); await upsertAdsAccount(); - setCreatingWhich( null ); } else if ( which === CREATING_MC_ACCOUNT ) { await handleCreateAccount(); - setCreatingWhich( null ); } else if ( which === CREATING_ADS_ACCOUNT ) { await upsertAdsAccount(); - setCreatingWhich( null ); } + setCreatingWhich( null ); }; handleCreateAccountCallback(); diff --git a/js/src/hooks/useAutoCreateAdsMCAccounts.test.js b/js/src/hooks/useAutoCreateAdsMCAccounts.test.js index bad8d3a858..84c3ce5da5 100644 --- a/js/src/hooks/useAutoCreateAdsMCAccounts.test.js +++ b/js/src/hooks/useAutoCreateAdsMCAccounts.test.js @@ -66,9 +66,11 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { const { result } = renderHook( () => useAutoCreateAdsMCAccounts() ); await act( async () => { - // It should create both accounts. expect( result.current.creatingWhich ).toBe( 'both' ); } ); + + expect( handleCreateAccount ).toHaveBeenCalledTimes( 1 ); + expect( upsertAdsAccount ).toHaveBeenCalledTimes( 1 ); } ); it( 'should create only the Merchant Center account', async () => { @@ -83,11 +85,12 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { } ); const { result } = renderHook( () => useAutoCreateAdsMCAccounts() ); - // It should create only the Merchant Center account. await act( async () => { - // It should create both accounts. expect( result.current.creatingWhich ).toBe( 'mc' ); } ); + + expect( handleCreateAccount ).toHaveBeenCalledTimes( 1 ); + expect( upsertAdsAccount ).toHaveBeenCalledTimes( 0 ); } ); it( 'should create only the Google Ads account', async () => { @@ -102,16 +105,17 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { } ); const { result } = renderHook( () => useAutoCreateAdsMCAccounts() ); - // It should create only the Google Ads account. await act( async () => { expect( result.current.creatingWhich ).toBe( 'ads' ); } ); + + expect( handleCreateAccount ).toHaveBeenCalledTimes( 0 ); + expect( upsertAdsAccount ).toHaveBeenCalledTimes( 1 ); } ); } ); describe( 'Existing accounts', () => { beforeEach( () => { - // Existing Ads and MC accounts. useExistingGoogleAdsAccounts.mockReturnValue( { hasFinishedResolution: true, existingAccounts: [ @@ -136,10 +140,7 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { it( 'should not create accounts if they already exist', () => { const { result } = renderHook( () => useAutoCreateAdsMCAccounts() ); - // It should not create any accounts. expect( result.current.creatingWhich ).toBe( null ); - - // make sure functions are not called. expect( handleCreateAccount ).not.toHaveBeenCalled(); expect( upsertAdsAccount ).not.toHaveBeenCalled(); } ); diff --git a/js/src/hooks/useGoogleAdsAccountReady.js b/js/src/hooks/useGoogleAdsAccountReady.js index 0b6de9cd4a..cb95e06145 100644 --- a/js/src/hooks/useGoogleAdsAccountReady.js +++ b/js/src/hooks/useGoogleAdsAccountReady.js @@ -4,18 +4,22 @@ import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import useGoogleAdsAccountStatus from '.~/hooks/useGoogleAdsAccountStatus'; +/** + * Hook to check if the Google Ads account is ready. + * This is used to determine if the user can proceed to the next step. + * + * @return {boolean|null} Whether the Google Ads account is ready. `null` if the state is not yet determined. + */ const useGoogleAdsAccountReady = () => { const { hasGoogleAdsConnection } = useGoogleAdsAccount(); const { hasAccess, step } = useGoogleAdsAccountStatus(); - // Ads is ready when we have a connection and verified and verified access. - // Billing is not required, and the 'link_merchant' step will be resolved - // when the MC the account is connected. - return ( + const isReady = hasGoogleAdsConnection && hasAccess && - [ '', 'billing', 'link_merchant' ].includes( step ) - ); + [ '', 'billing', 'link_merchant' ].includes( step ); + + return isReady !== null ? isReady : null; }; export default useGoogleAdsAccountReady; diff --git a/js/src/hooks/useGoogleMCAccount.js b/js/src/hooks/useGoogleMCAccount.js index 255943f1a3..8dbf3a0fff 100644 --- a/js/src/hooks/useGoogleMCAccount.js +++ b/js/src/hooks/useGoogleMCAccount.js @@ -18,6 +18,8 @@ import { GOOGLE_MC_ACCOUNT_STATUS } from '.~/constants'; * @property {boolean} isResolving Whether resolution is in progress. * @property {boolean} hasFinishedResolution Whether resolution has completed. * @property {boolean} isPreconditionReady Whether the precondition of continued connection processing is fulfilled. + * @property {boolean} hasGoogleMCConnection Whether the user has a Google Merchant Center account connection established. + * @property {boolean} isReady Whether the user has a Google Merchant Center account is in connected state. */ const googleMCAccountSelector = 'getGoogleMCAccount'; @@ -46,6 +48,8 @@ const useGoogleMCAccount = () => { // has not been granted necessary access permissions for Google Merchant Center, then // the precondition doesn't meet. isPreconditionReady: false, + hasGoogleMCConnection: false, + isReady: false, }; } @@ -60,6 +64,12 @@ const useGoogleMCAccount = () => { GOOGLE_MC_ACCOUNT_STATUS.INCOMPLETE, ].includes( acc?.status ); + const isReady = + hasGoogleMCConnection && + ( acc.status === GOOGLE_MC_ACCOUNT_STATUS.CONNECTED || + ( acc.status === GOOGLE_MC_ACCOUNT_STATUS.INCOMPLETE && + acc?.step === 'link_ads' ) ); + return { googleMCAccount: acc, isResolving: isResolvingGoogleMCAccount, @@ -68,6 +78,7 @@ const useGoogleMCAccount = () => { ), isPreconditionReady: true, hasGoogleMCConnection, + isReady, }; }, [ diff --git a/js/src/hooks/useGoogleMCAccountReady.js b/js/src/hooks/useGoogleMCAccountReady.js deleted file mode 100644 index 1a2505620d..0000000000 --- a/js/src/hooks/useGoogleMCAccountReady.js +++ /dev/null @@ -1,18 +0,0 @@ -/** - * Internal dependencies - */ -import useGoogleMCAccount from './useGoogleMCAccount'; - -const useGoogleMCAccountReady = () => { - const { hasGoogleMCConnection, googleMCAccount } = useGoogleMCAccount(); - - // MC is ready when we have a connection and preconditions are met. - // The `link_ads` step will be resolved when the Ads account is connected - // since these can be connected in any order. - return ( - hasGoogleMCConnection && - [ '', 'link_ads' ].includes( googleMCAccount?.step ) - ); -}; - -export default useGoogleMCAccountReady; diff --git a/js/src/setup-mc/setup-stepper/setup-accounts/index.js b/js/src/setup-mc/setup-stepper/setup-accounts/index.js index 69cbb4588c..cf200ed4df 100644 --- a/js/src/setup-mc/setup-stepper/setup-accounts/index.js +++ b/js/src/setup-mc/setup-stepper/setup-accounts/index.js @@ -25,7 +25,6 @@ import Faqs from './faqs'; import './index.scss'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import useGoogleAdsAccountReady from '.~/hooks/useGoogleAdsAccountReady'; -import useGoogleMCAccountReady from '.~/hooks/useGoogleMCAccountReady'; /** * Renders the disclaimer of Comparison Shopping Service (CSS). @@ -84,11 +83,13 @@ const SetupAccounts = ( props ) => { const { onContinue = () => {} } = props; const { jetpack } = useJetpackAccount(); const { google, scope } = useGoogleAccount(); - const { googleMCAccount, isPreconditionReady: isGMCPreconditionReady } = - useGoogleMCAccount(); + const { + googleMCAccount, + isPreconditionReady: isGMCPreconditionReady, + isReady: isGoogleMCReady, + } = useGoogleMCAccount(); const { hasFinishedResolution } = useGoogleAdsAccount(); const isGoogleAdsReady = useGoogleAdsAccountReady(); - const isGoogleMCReady = useGoogleMCAccountReady(); /** * When jetpack is loading, or when google account is loading,