From 1535369c24e80d6e47031e60aade49c76731cfc3 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Wed, 18 Sep 2024 20:55:16 +0530 Subject: [PATCH 01/27] Initial structure. --- .../connected-google-combo-account-card.js | 57 +++++++++++++++++++ .../google-combo-account-card/index.js | 3 +- 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 js/src/components/google-combo-account-card/connected-google-combo-account-card.js 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 new file mode 100644 index 0000000000..674533122f --- /dev/null +++ b/js/src/components/google-combo-account-card/connected-google-combo-account-card.js @@ -0,0 +1,57 @@ + +/** + * External dependencies + */ +import { __ } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import AccountCard, { APPEARANCE } from '../account-card'; +import AppButton from '../app-button'; +import ConnectedIconLabel from '../connected-icon-label'; +import Section from '../../wcdl/section'; +import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts'; +import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; + +/** + * Renders a Google account card UI with connected account information. + * It will also kickoff Ads and Merchant Center account creation if the user does not have accounts. + * + * @param {Object} props React props. + * @param {{ email: string }} props.googleAccount A data payload object containing the user's Google account email. + * + * @fires gla_google_account_connect_different_account_button_click + */ +const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { + const { data: existingMCAccounts, isResolving: MCAccountsResolving } = useExistingGoogleMCAccounts(); + const { existingAccounts: existingAdsAccount, isResolving: AdsAccountsResolving } = useExistingGoogleAdsAccounts(); + + console.log('MCAccountsResolving', MCAccountsResolving); + console.log('existingMCAccount', existingMCAccounts); + //console.log('AdsAccountsResolving', AdsAccountsResolving); + //console.log('existingAdsAccount', existingAdsAccount); + + return ( + } + > + + {} } + /> + + + ); +}; + +export default ConnectedGoogleComboAccountCard; diff --git a/js/src/components/google-combo-account-card/index.js b/js/src/components/google-combo-account-card/index.js index 098fb7b4ef..459234ca60 100644 --- a/js/src/components/google-combo-account-card/index.js +++ b/js/src/components/google-combo-account-card/index.js @@ -7,6 +7,7 @@ import AccountCard from '.~/components/account-card'; import RequestFullAccessGoogleAccountCard from '../google-account-card/request-full-access-google-account-card'; import { ConnectedGoogleAccountCard } from '../google-account-card'; import ConnectGoogleComboAccountCard from './connect-google-combo-account-card'; +import ConnectedGoogleComboAccountCard from './connected-google-combo-account-card'; export default function GoogleComboAccountCard( { disabled = false } ) { const { google, scope, hasFinishedResolution } = useGoogleAccount(); @@ -18,7 +19,7 @@ export default function GoogleComboAccountCard( { disabled = false } ) { const isConnected = google?.active === 'yes'; if ( isConnected && scope.glaRequired ) { - return ; + return ; } if ( isConnected && ! scope.glaRequired ) { From 0d7965c7aa10672c56000a54137f4a6662c595d0 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 19 Sep 2024 16:20:41 +0530 Subject: [PATCH 02/27] Use hooks to determine existing accounts. --- .../connected-google-combo-account-card.js | 56 +++++++++---------- .../create-mc-ads-accounts/index.js | 41 ++++++++++++++ .../google-combo-account-card/index.js | 21 ++++++- .../useCreateMCAccount.js | 2 + js/src/hooks/useApiFetchCallback.js | 2 + 5 files changed, 89 insertions(+), 33 deletions(-) create mode 100644 js/src/components/google-combo-account-card/create-mc-ads-accounts/index.js 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 674533122f..7ef4e3a359 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 @@ -1,55 +1,51 @@ - /** * External dependencies */ +import { createInterpolateElement } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ import AccountCard, { APPEARANCE } from '../account-card'; -import AppButton from '../app-button'; -import ConnectedIconLabel from '../connected-icon-label'; -import Section from '../../wcdl/section'; -import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts'; -import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; +import CreateMCAdsAccounts from './create-mc-ads-accounts'; + +/** + * Clicking on the "connect to a different Google account" button. + * + * @event gla_google_account_connect_different_account_button_click + */ /** * Renders a Google account card UI with connected account information. * It will also kickoff Ads and Merchant Center account creation if the user does not have accounts. * * @param {Object} props React props. - * @param {{ email: string }} props.googleAccount A data payload object containing the user's Google account email. + * @param {{ MCAccount: string }} props.MCAccount The Google Merchant Center account. + * @param {{ AdsAccounts: string }} props.AdsAccounts The Google Ads accounts. * * @fires gla_google_account_connect_different_account_button_click */ -const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { - const { data: existingMCAccounts, isResolving: MCAccountsResolving } = useExistingGoogleMCAccounts(); - const { existingAccounts: existingAdsAccount, isResolving: AdsAccountsResolving } = useExistingGoogleAdsAccounts(); - - console.log('MCAccountsResolving', MCAccountsResolving); - console.log('existingMCAccount', existingMCAccounts); - //console.log('AdsAccountsResolving', AdsAccountsResolving); - //console.log('existingAdsAccount', existingAdsAccount); - +const ConnectedGoogleComboAccountCard = ( { MCAccounts, AdsAccounts } ) => { return ( } + description={ __( + 'You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you.', + 'google-listings-and-ads' + ) } + helper={ createInterpolateElement( + __( + '

Merchant Center is required to sync products so they show on Google. Google Ads is required to set up conversion measurement for your store.

', + 'google-listings-and-ads' + ), + { + p:

, + } + ) } + indicator={ __( 'Creating…', 'google-listings-and-ads' ) } > - - {} } - /> - + { MCAccounts.length === 0 && AdsAccounts.length === 0 && } ); }; diff --git a/js/src/components/google-combo-account-card/create-mc-ads-accounts/index.js b/js/src/components/google-combo-account-card/create-mc-ads-accounts/index.js new file mode 100644 index 0000000000..b57913af14 --- /dev/null +++ b/js/src/components/google-combo-account-card/create-mc-ads-accounts/index.js @@ -0,0 +1,41 @@ +/** + * External dependencies + */ +import { useEffect, useState } from '@wordpress/element'; +import { __ } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import useCreateMCAccount from '../../google-mc-account-card/useCreateMCAccount'; +import useUpsertAdsAccount from '../../../hooks/useUpsertAdsAccount'; + +/** + * Create MC and Ads accounts. + */ +const CreateMCAdsAccounts = () => { + const [merchantCenterAccountCreated, setMerchantCenterAccountCreated] = useState( false ); + const [googleAdsAccountCreated, setGoogleAdsAccountCreated] = useState( false ); + + const [ handleCreateMCAccount, { loading, error, response } ] = useCreateMCAccount(); + + useEffect( () => { + if ( ! loading && ! merchantCenterAccountCreated ) { + handleCreateMCAccount(); + setMerchantCenterAccountCreated( true ); + } + + console.log( 'response', response ); + console.log( 'error', error ); + console.log( 'loading', loading ); + }, [ + loading, + response, + error, + merchantCenterAccountCreated, + ]); + + return
Creating MC and Ads account.
+}; + +export default CreateMCAdsAccounts; diff --git a/js/src/components/google-combo-account-card/index.js b/js/src/components/google-combo-account-card/index.js index 459234ca60..ae03d8519d 100644 --- a/js/src/components/google-combo-account-card/index.js +++ b/js/src/components/google-combo-account-card/index.js @@ -5,21 +5,36 @@ import useGoogleAccount from '.~/hooks/useGoogleAccount'; import AppSpinner from '.~/components/app-spinner'; import AccountCard from '.~/components/account-card'; import RequestFullAccessGoogleAccountCard from '../google-account-card/request-full-access-google-account-card'; -import { ConnectedGoogleAccountCard } from '../google-account-card'; import ConnectGoogleComboAccountCard from './connect-google-combo-account-card'; import ConnectedGoogleComboAccountCard from './connected-google-combo-account-card'; +import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts'; +import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; export default function GoogleComboAccountCard( { disabled = false } ) { const { google, scope, hasFinishedResolution } = useGoogleAccount(); + /* const { data: existingMCAccounts, isResolving: MCAccountsResolving } = + useExistingGoogleMCAccounts(); + const { + existingAccounts: existingAdsAccount, + isResolving: AdsAccountsResolving, + } = useExistingGoogleAdsAccounts(); */ - if ( ! hasFinishedResolution ) { + if ( + ! hasFinishedResolution + ) { return } />; } const isConnected = google?.active === 'yes'; if ( isConnected && scope.glaRequired ) { - return ; + return ( + + ); } if ( isConnected && ! scope.glaRequired ) { diff --git a/js/src/components/google-mc-account-card/useCreateMCAccount.js b/js/src/components/google-mc-account-card/useCreateMCAccount.js index a503f7354d..7af7d4d7e0 100644 --- a/js/src/components/google-mc-account-card/useCreateMCAccount.js +++ b/js/src/components/google-mc-account-card/useCreateMCAccount.js @@ -18,6 +18,8 @@ const useCreateMCAccount = () => { method: 'POST', } ); + console.log( 'result', result ); + const handleCreateAccount = async () => { try { await fetchCreateMCAccount( { diff --git a/js/src/hooks/useApiFetchCallback.js b/js/src/hooks/useApiFetchCallback.js index 433a2ec4f8..886bda107d 100644 --- a/js/src/hooks/useApiFetchCallback.js +++ b/js/src/hooks/useApiFetchCallback.js @@ -143,6 +143,8 @@ const useApiFetchCallback = ( options, initialState = defaultState ) => { parse: false, } ); + console.log( 'useApiFetchCallback response', response ); + const responseClone = response.clone(); const data = responseClone.json && ( await responseClone.json() ); From 9e9b662f6afd26498b54c1f862091ce03819c28c Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 19 Sep 2024 20:35:23 +0530 Subject: [PATCH 03/27] Fix lint. --- .../connected-google-combo-account-card.js | 98 ++++++++++++++----- .../create-accounts/index.js | 55 +++++++++++ .../create-mc-ads-accounts/index.js | 41 -------- .../google-combo-account-card/index.js | 15 ++- .../useCreateMCAccount.js | 2 - js/src/hooks/useApiFetchCallback.js | 2 - tests/e2e/test-snippets/test-snippets.php | 4 + 7 files changed, 139 insertions(+), 78 deletions(-) create mode 100644 js/src/components/google-combo-account-card/create-accounts/index.js delete mode 100644 js/src/components/google-combo-account-card/create-mc-ads-accounts/index.js 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 7ef4e3a359..9cef4da0ea 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 @@ -1,14 +1,18 @@ /** * External dependencies */ -import { createInterpolateElement } from '@wordpress/element'; +import { + createInterpolateElement, + useEffect, + useState, +} from '@wordpress/element'; import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ import AccountCard, { APPEARANCE } from '../account-card'; -import CreateMCAdsAccounts from './create-mc-ads-accounts'; +import CreateAccounts from './create-accounts'; /** * Clicking on the "connect to a different Google account" button. @@ -21,33 +25,77 @@ import CreateMCAdsAccounts from './create-mc-ads-accounts'; * It will also kickoff Ads and Merchant Center account creation if the user does not have accounts. * * @param {Object} props React props. - * @param {{ MCAccount: string }} props.MCAccount The Google Merchant Center account. + * @param {{ googleAccount: object }} props.googleAccount The Google account. + * @param {{ MCAccounts: string }} props.MCAccounts The Google Merchant Center account. * @param {{ AdsAccounts: string }} props.AdsAccounts The Google Ads accounts. * * @fires gla_google_account_connect_different_account_button_click */ -const ConnectedGoogleComboAccountCard = ( { MCAccounts, AdsAccounts } ) => { - return ( - Merchant Center is required to sync products so they show on Google. Google Ads is required to set up conversion measurement for your store.

', - 'google-listings-and-ads' - ), - { - p:

, - } - ) } - indicator={ __( 'Creating…', 'google-listings-and-ads' ) } - > - { MCAccounts.length === 0 && AdsAccounts.length === 0 && } - - ); +const ConnectedGoogleComboAccountCard = ( { + googleAccount, + MCAccounts, + AdsAccounts, +} ) => { + const [ accounts, setAccounts ] = useState( { + MCAccounts, + AdsAccounts, + } ); + + useEffect( () => { + if ( MCAccounts.length ) { + setAccounts( { + ...accounts, + MCAccounts, + } ); + } + + if ( AdsAccounts.length ) { + setAccounts( { + ...accounts, + AdsAccounts, + } ); + } + }, [ MCAccounts, AdsAccounts, accounts ] ); + + const existingAccounts = Object.keys( accounts ).some( + ( account ) => accounts[ account ].length > 0 + ); + + const description = ! existingAccounts + ? createInterpolateElement( + __( + '

You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you.

', + 'google-listings-and-ads' + ), + { + p:

, + } + ) + : ''; + + return ( + Merchant Center is required to sync products so they show on Google. Google Ads is required to set up conversion measurement for your store.

', + 'google-listings-and-ads' + ), + { + p:

, + } + ) } + indicator={ 'Creating...' } + > + { ! existingAccounts && ( + + ) } + + ); }; export default ConnectedGoogleComboAccountCard; diff --git a/js/src/components/google-combo-account-card/create-accounts/index.js b/js/src/components/google-combo-account-card/create-accounts/index.js new file mode 100644 index 0000000000..ce0950221f --- /dev/null +++ b/js/src/components/google-combo-account-card/create-accounts/index.js @@ -0,0 +1,55 @@ +/** + * External dependencies + */ +import { useEffect } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import useCreateMCAccount from '../../google-mc-account-card/useCreateMCAccount'; +import useUpsertAdsAccount from '../../../hooks/useUpsertAdsAccount'; +import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; + +/** + * Create MC and Ads accounts. + */ +const CreateAccounts = ( { setAccounts } ) => { + const { googleAdsAccount } = useGoogleAdsAccount(); + + const [ handleCreateAccount, { loading, response } ] = useCreateMCAccount(); + const [ , { loading: adsAccountsLoading } ] = useUpsertAdsAccount(); + + useEffect( () => { + console.log( 'googleAdsAccount', googleAdsAccount ); + console.log( 'response', response ); + + if ( + ! loading && + ! adsAccountsLoading && + response && + response.status === 200 + ) { + const createMCAccount = async () => { + await handleCreateAccount(); + }; + + createMCAccount(); + + setAccounts( { + MCAccounts: [ response.data ], + AdsAccounts: [ googleAdsAccount.id ], + } ); + } + }, [ + loading, + adsAccountsLoading, + response, + handleCreateAccount, + googleAdsAccount, + setAccounts, + ] ); + + return null; +}; + +export default CreateAccounts; diff --git a/js/src/components/google-combo-account-card/create-mc-ads-accounts/index.js b/js/src/components/google-combo-account-card/create-mc-ads-accounts/index.js deleted file mode 100644 index b57913af14..0000000000 --- a/js/src/components/google-combo-account-card/create-mc-ads-accounts/index.js +++ /dev/null @@ -1,41 +0,0 @@ -/** - * External dependencies - */ -import { useEffect, useState } from '@wordpress/element'; -import { __ } from '@wordpress/i18n'; - -/** - * Internal dependencies - */ -import useCreateMCAccount from '../../google-mc-account-card/useCreateMCAccount'; -import useUpsertAdsAccount from '../../../hooks/useUpsertAdsAccount'; - -/** - * Create MC and Ads accounts. - */ -const CreateMCAdsAccounts = () => { - const [merchantCenterAccountCreated, setMerchantCenterAccountCreated] = useState( false ); - const [googleAdsAccountCreated, setGoogleAdsAccountCreated] = useState( false ); - - const [ handleCreateMCAccount, { loading, error, response } ] = useCreateMCAccount(); - - useEffect( () => { - if ( ! loading && ! merchantCenterAccountCreated ) { - handleCreateMCAccount(); - setMerchantCenterAccountCreated( true ); - } - - console.log( 'response', response ); - console.log( 'error', error ); - console.log( 'loading', loading ); - }, [ - loading, - response, - error, - merchantCenterAccountCreated, - ]); - - return
Creating MC and Ads account.
-}; - -export default CreateMCAdsAccounts; diff --git a/js/src/components/google-combo-account-card/index.js b/js/src/components/google-combo-account-card/index.js index ae03d8519d..c76f2ff5a7 100644 --- a/js/src/components/google-combo-account-card/index.js +++ b/js/src/components/google-combo-account-card/index.js @@ -12,15 +12,14 @@ import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; export default function GoogleComboAccountCard( { disabled = false } ) { const { google, scope, hasFinishedResolution } = useGoogleAccount(); - /* const { data: existingMCAccounts, isResolving: MCAccountsResolving } = - useExistingGoogleMCAccounts(); - const { - existingAccounts: existingAdsAccount, - isResolving: AdsAccountsResolving, - } = useExistingGoogleAdsAccounts(); */ + const { isResolving: MCAccountsResolving } = useExistingGoogleMCAccounts(); + const { isResolving: AdsAccountsResolving } = + useExistingGoogleAdsAccounts(); if ( - ! hasFinishedResolution + ! hasFinishedResolution || + MCAccountsResolving || + AdsAccountsResolving ) { return } />; } @@ -31,7 +30,7 @@ export default function GoogleComboAccountCard( { disabled = false } ) { return ( ); diff --git a/js/src/components/google-mc-account-card/useCreateMCAccount.js b/js/src/components/google-mc-account-card/useCreateMCAccount.js index 7af7d4d7e0..a503f7354d 100644 --- a/js/src/components/google-mc-account-card/useCreateMCAccount.js +++ b/js/src/components/google-mc-account-card/useCreateMCAccount.js @@ -18,8 +18,6 @@ const useCreateMCAccount = () => { method: 'POST', } ); - console.log( 'result', result ); - const handleCreateAccount = async () => { try { await fetchCreateMCAccount( { diff --git a/js/src/hooks/useApiFetchCallback.js b/js/src/hooks/useApiFetchCallback.js index 886bda107d..433a2ec4f8 100644 --- a/js/src/hooks/useApiFetchCallback.js +++ b/js/src/hooks/useApiFetchCallback.js @@ -143,8 +143,6 @@ const useApiFetchCallback = ( options, initialState = defaultState ) => { parse: false, } ); - console.log( 'useApiFetchCallback response', response ); - const responseClone = response.clone(); const data = responseClone.json && ( await responseClone.json() ); diff --git a/tests/e2e/test-snippets/test-snippets.php b/tests/e2e/test-snippets/test-snippets.php index ebb9b0f710..9590895767 100644 --- a/tests/e2e/test-snippets/test-snippets.php +++ b/tests/e2e/test-snippets/test-snippets.php @@ -40,3 +40,7 @@ function ( array $value_options ) { return $value_options; } ); + +add_filter( 'woocommerce_gla_ads_billing_setup_status', function( $status ) { + return 'approved'; +} ); From c5023b6b37874c7bfdb9f123029c99f8079f73e0 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Fri, 20 Sep 2024 14:24:50 +0530 Subject: [PATCH 04/27] Fix: CreateAccounts component. Multiple API calls. --- .../connected-google-combo-account-card.js | 108 +++++++++++------- .../create-accounts/index.js | 55 ++++----- .../google-combo-account-card/index.js | 11 +- 3 files changed, 88 insertions(+), 86 deletions(-) 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 9cef4da0ea..aa95517b4d 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 @@ -1,18 +1,19 @@ /** * External dependencies */ -import { - createInterpolateElement, - useEffect, - useState, -} from '@wordpress/element'; -import { __ } from '@wordpress/i18n'; +import { createInterpolateElement, useState } from '@wordpress/element'; +import { __, sprintf } from '@wordpress/i18n'; /** * Internal dependencies */ import AccountCard, { APPEARANCE } from '../account-card'; import CreateAccounts from './create-accounts'; +import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts'; +import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; +import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; +import useGoogleMCAccount from '.~/hooks/useGoogleMCAccount'; +import AppSpinner from '../app-spinner'; /** * Clicking on the "connect to a different Google account" button. @@ -26,43 +27,43 @@ import CreateAccounts from './create-accounts'; * * @param {Object} props React props. * @param {{ googleAccount: object }} props.googleAccount The Google account. - * @param {{ MCAccounts: string }} props.MCAccounts The Google Merchant Center account. - * @param {{ AdsAccounts: string }} props.AdsAccounts The Google Ads accounts. * * @fires gla_google_account_connect_different_account_button_click */ -const ConnectedGoogleComboAccountCard = ( { - googleAccount, - MCAccounts, - AdsAccounts, -} ) => { - const [ accounts, setAccounts ] = useState( { - MCAccounts, - AdsAccounts, - } ); +const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { + const [ isCreatingAccounts, setIsCreatingAccounts ] = useState( false ); - useEffect( () => { - if ( MCAccounts.length ) { - setAccounts( { - ...accounts, - MCAccounts, - } ); - } + const { + data: existingMCAccounts, + hasFinishedResolution: hasFinishedResolutionForExistingMCAccounts, + } = useExistingGoogleMCAccounts(); + const { + existingAccounts: existingAdsAccount, + isResolving: isResolvingExistingAdsAccount, + } = useExistingGoogleAdsAccounts(); - if ( AdsAccounts.length ) { - setAccounts( { - ...accounts, - AdsAccounts, - } ); - } - }, [ MCAccounts, AdsAccounts, accounts ] ); + const { + googleMCAccount, + hasFinishedResolution: hasFinishedResolutionForCurrentMCAccount, + } = useGoogleMCAccount(); + const { + googleAdsAccount, + hasFinishedResolution: hasFinishedResolutionForCurrentAdsAccount, + } = useGoogleAdsAccount(); - const existingAccounts = Object.keys( accounts ).some( - ( account ) => accounts[ account ].length > 0 - ); + if ( + ! hasFinishedResolutionForExistingMCAccounts || + isResolvingExistingAdsAccount + ) { + return } />; + } - const description = ! existingAccounts - ? createInterpolateElement( + const existingAccounts = + existingMCAccounts?.length || existingAdsAccount?.length; + + const Description = () => { + if ( isCreatingAccounts ) { + return createInterpolateElement( __( '

You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you.

', 'google-listings-and-ads' @@ -70,13 +71,37 @@ const ConnectedGoogleComboAccountCard = ( { { p:

, } - ) - : ''; + ); + } + + return ( + <> +

{ googleAccount?.email }

+

+ { sprintf( + // Translators: %s is the Merchant Center ID + __( + 'Merchant Center ID: %s', + 'google-listings-and-ads' + ), + googleMCAccount?.id + ) } +

+

+ { sprintf( + // Translators: %s is the Google Ads ID + __( 'Google Ads ID: %s', 'google-listings-and-ads' ), + googleAdsAccount?.id + ) } +

+ + ); + }; return ( } helper={ createInterpolateElement( __( '

Merchant Center is required to sync products so they show on Google. Google Ads is required to set up conversion measurement for your store.

', @@ -86,12 +111,11 @@ const ConnectedGoogleComboAccountCard = ( { p:

, } ) } - indicator={ 'Creating...' } + indicator={ isCreatingAccounts ? 'Creating...' : null } > { ! existingAccounts && ( ) } diff --git a/js/src/components/google-combo-account-card/create-accounts/index.js b/js/src/components/google-combo-account-card/create-accounts/index.js index ce0950221f..d044197834 100644 --- a/js/src/components/google-combo-account-card/create-accounts/index.js +++ b/js/src/components/google-combo-account-card/create-accounts/index.js @@ -7,47 +7,34 @@ import { useEffect } from '@wordpress/element'; * Internal dependencies */ import useCreateMCAccount from '../../google-mc-account-card/useCreateMCAccount'; -import useUpsertAdsAccount from '../../../hooks/useUpsertAdsAccount'; -import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; +import useUpsertAdsAccount from '.~/hooks/useUpsertAdsAccount'; +import { receiveMCAccount } from '.~/data/actions'; /** * Create MC and Ads accounts. */ -const CreateAccounts = ( { setAccounts } ) => { - const { googleAdsAccount } = useGoogleAdsAccount(); +const CreateAccounts = ( { setIsCreatingAccounts } ) => { + const [ handleCreateAccount, { data: account, response } ] = + useCreateMCAccount(); - const [ handleCreateAccount, { loading, response } ] = useCreateMCAccount(); - const [ , { loading: adsAccountsLoading } ] = useUpsertAdsAccount(); + const [ upsertAdsAccount ] = useUpsertAdsAccount(); + + if ( response?.status === 200 ) { + receiveMCAccount( account ); + setIsCreatingAccounts( false ); + } useEffect( () => { - console.log( 'googleAdsAccount', googleAdsAccount ); - console.log( 'response', response ); - - if ( - ! loading && - ! adsAccountsLoading && - response && - response.status === 200 - ) { - const createMCAccount = async () => { - await handleCreateAccount(); - }; - - createMCAccount(); - - setAccounts( { - MCAccounts: [ response.data ], - AdsAccounts: [ googleAdsAccount.id ], - } ); - } - }, [ - loading, - adsAccountsLoading, - response, - handleCreateAccount, - googleAdsAccount, - setAccounts, - ] ); + setIsCreatingAccounts( true ); + + const createAccounts = async () => { + await handleCreateAccount(); + await upsertAdsAccount(); + }; + + createAccounts(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [] ); return null; }; diff --git a/js/src/components/google-combo-account-card/index.js b/js/src/components/google-combo-account-card/index.js index c76f2ff5a7..cf5219a5f0 100644 --- a/js/src/components/google-combo-account-card/index.js +++ b/js/src/components/google-combo-account-card/index.js @@ -7,20 +7,11 @@ import AccountCard from '.~/components/account-card'; import RequestFullAccessGoogleAccountCard from '../google-account-card/request-full-access-google-account-card'; import ConnectGoogleComboAccountCard from './connect-google-combo-account-card'; import ConnectedGoogleComboAccountCard from './connected-google-combo-account-card'; -import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts'; -import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; export default function GoogleComboAccountCard( { disabled = false } ) { const { google, scope, hasFinishedResolution } = useGoogleAccount(); - const { isResolving: MCAccountsResolving } = useExistingGoogleMCAccounts(); - const { isResolving: AdsAccountsResolving } = - useExistingGoogleAdsAccounts(); - if ( - ! hasFinishedResolution || - MCAccountsResolving || - AdsAccountsResolving - ) { + if ( ! hasFinishedResolution ) { return } />; } From d41d82508776e01fc903630a2ad5d4999ed9df09 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Fri, 20 Sep 2024 14:26:37 +0530 Subject: [PATCH 05/27] Remove redundant props. --- js/src/components/google-combo-account-card/index.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/js/src/components/google-combo-account-card/index.js b/js/src/components/google-combo-account-card/index.js index cf5219a5f0..2bc42e0395 100644 --- a/js/src/components/google-combo-account-card/index.js +++ b/js/src/components/google-combo-account-card/index.js @@ -18,13 +18,7 @@ export default function GoogleComboAccountCard( { disabled = false } ) { const isConnected = google?.active === 'yes'; if ( isConnected && scope.glaRequired ) { - return ( - - ); + return ; } if ( isConnected && ! scope.glaRequired ) { From 41e6d04ea7e2de376a18e2db7115ad59509f4981 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Mon, 23 Sep 2024 13:08:42 +0530 Subject: [PATCH 06/27] Remove redundant code. --- tests/e2e/test-snippets/test-snippets.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/e2e/test-snippets/test-snippets.php b/tests/e2e/test-snippets/test-snippets.php index 9590895767..ebb9b0f710 100644 --- a/tests/e2e/test-snippets/test-snippets.php +++ b/tests/e2e/test-snippets/test-snippets.php @@ -40,7 +40,3 @@ function ( array $value_options ) { return $value_options; } ); - -add_filter( 'woocommerce_gla_ads_billing_setup_status', function( $status ) { - return 'approved'; -} ); From 691733719effdb8f0c8c646f4d82938d4758132f Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Mon, 23 Sep 2024 23:21:43 +0530 Subject: [PATCH 07/27] Introduce hook. --- .../connect-google-combo-account-card.scss | 7 ++ .../connected-google-combo-account-card.js | 37 +++----- .../create-accounts/index.js | 42 --------- .../useCreateAccounts.js | 87 +++++++++++++++++++ 4 files changed, 104 insertions(+), 69 deletions(-) delete mode 100644 js/src/components/google-combo-account-card/create-accounts/index.js create mode 100644 js/src/components/google-combo-account-card/useCreateAccounts.js diff --git a/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss b/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss index 9260eecaf1..0f29e4ba01 100644 --- a/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss +++ b/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss @@ -3,4 +3,11 @@ .gla-account-card__helper { font-size: $gla-font-base; } + + &.gla-connected-google-combo-account-card { + + .gla-account-card__description { + gap: 0; + } + } } 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 aa95517b4d..6ef60d03a2 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 @@ -1,19 +1,17 @@ /** * External dependencies */ -import { createInterpolateElement, useState } from '@wordpress/element'; +import { createInterpolateElement } from '@wordpress/element'; import { __, sprintf } from '@wordpress/i18n'; /** * Internal dependencies */ import AccountCard, { APPEARANCE } from '../account-card'; -import CreateAccounts from './create-accounts'; -import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts'; -import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import useGoogleMCAccount from '.~/hooks/useGoogleMCAccount'; import AppSpinner from '../app-spinner'; +import useCreateAccounts from './useCreateAccounts'; /** * Clicking on the "connect to a different Google account" button. @@ -31,36 +29,26 @@ import AppSpinner from '../app-spinner'; * @fires gla_google_account_connect_different_account_button_click */ const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { - const [ isCreatingAccounts, setIsCreatingAccounts ] = useState( false ); - - const { - data: existingMCAccounts, - hasFinishedResolution: hasFinishedResolutionForExistingMCAccounts, - } = useExistingGoogleMCAccounts(); - const { - existingAccounts: existingAdsAccount, - isResolving: isResolvingExistingAdsAccount, - } = useExistingGoogleAdsAccounts(); - const { googleMCAccount, hasFinishedResolution: hasFinishedResolutionForCurrentMCAccount, } = useGoogleMCAccount(); + const { googleAdsAccount, hasFinishedResolution: hasFinishedResolutionForCurrentAdsAccount, } = useGoogleAdsAccount(); + const { accountCreationResolved, isCreatingAccounts } = useCreateAccounts(); + if ( - ! hasFinishedResolutionForExistingMCAccounts || - isResolvingExistingAdsAccount + ! hasFinishedResolutionForCurrentMCAccount || + ! hasFinishedResolutionForCurrentAdsAccount || + ! accountCreationResolved ) { return } />; } - const existingAccounts = - existingMCAccounts?.length || existingAdsAccount?.length; - const Description = () => { if ( isCreatingAccounts ) { return createInterpolateElement( @@ -101,6 +89,7 @@ const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { return ( } helper={ createInterpolateElement( __( @@ -112,13 +101,7 @@ const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { } ) } indicator={ isCreatingAccounts ? 'Creating...' : null } - > - { ! existingAccounts && ( - - ) } - + /> ); }; diff --git a/js/src/components/google-combo-account-card/create-accounts/index.js b/js/src/components/google-combo-account-card/create-accounts/index.js deleted file mode 100644 index d044197834..0000000000 --- a/js/src/components/google-combo-account-card/create-accounts/index.js +++ /dev/null @@ -1,42 +0,0 @@ -/** - * External dependencies - */ -import { useEffect } from '@wordpress/element'; - -/** - * Internal dependencies - */ -import useCreateMCAccount from '../../google-mc-account-card/useCreateMCAccount'; -import useUpsertAdsAccount from '.~/hooks/useUpsertAdsAccount'; -import { receiveMCAccount } from '.~/data/actions'; - -/** - * Create MC and Ads accounts. - */ -const CreateAccounts = ( { setIsCreatingAccounts } ) => { - const [ handleCreateAccount, { data: account, response } ] = - useCreateMCAccount(); - - const [ upsertAdsAccount ] = useUpsertAdsAccount(); - - if ( response?.status === 200 ) { - receiveMCAccount( account ); - setIsCreatingAccounts( false ); - } - - useEffect( () => { - setIsCreatingAccounts( true ); - - const createAccounts = async () => { - await handleCreateAccount(); - await upsertAdsAccount(); - }; - - createAccounts(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [] ); - - return null; -}; - -export default CreateAccounts; diff --git a/js/src/components/google-combo-account-card/useCreateAccounts.js b/js/src/components/google-combo-account-card/useCreateAccounts.js new file mode 100644 index 0000000000..341e57ec6f --- /dev/null +++ b/js/src/components/google-combo-account-card/useCreateAccounts.js @@ -0,0 +1,87 @@ +/* eslint-disable react-hooks/exhaustive-deps */ +/** + * External dependencies + */ +import { useEffect, useRef } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import useCreateMCAccount from '../google-mc-account-card/useCreateMCAccount'; +import useUpsertAdsAccount from '.~/hooks/useUpsertAdsAccount'; +import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts'; +import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; +import { receiveMCAccount } from '.~/data/actions'; +import { useAppDispatch } from '.~/data'; +import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; + +/** + * Custom hook to handle the creation of MC and Ads accounts. + */ +const useCreateAccounts = () => { + const accountCreationResolvedRef = useRef( false ); + const isCreatingAccountsRef = useRef( false ); + + const { + googleAdsAccount, + hasFinishedResolution: hasFinishedResolutionForExistingAdsccounts, + } = useGoogleAdsAccount(); + + const { + data: existingMCAccounts, + hasFinishedResolution: hasFinishedResolutionForExistingMCAccounts, + } = useExistingGoogleMCAccounts(); + + const { + existingAccounts: existingAdsAccount, + isResolving: isResolvingExistingAdsAccount, + } = useExistingGoogleAdsAccounts(); + + const [ handleCreateAccount, { data: account, response } ] = + useCreateMCAccount(); + + const [ upsertAdsAccount, { loading } ] = useUpsertAdsAccount(); + const { invalidateResolution } = useAppDispatch(); + + // Process account creation completion. + useEffect( () => { + if ( response?.status === 200 && ! loading ) { + receiveMCAccount( account ); + invalidateResolution( 'getExistingGoogleAdsAccounts' ); + isCreatingAccountsRef.current = false; + } + }, [ response, loading ] ); + + useEffect( () => { + if ( + ! isResolvingExistingAdsAccount && + hasFinishedResolutionForExistingMCAccounts && + isCreatingAccountsRef.current === false && + accountCreationResolvedRef.current === false && + account === undefined + ) { + accountCreationResolvedRef.current = true; + const hasExistingAccounts = true; + + if ( ! hasExistingAccounts ) { + const createAccounts = async () => { + await handleCreateAccount(); + await upsertAdsAccount(); + }; + + isCreatingAccountsRef.current = true; + createAccounts(); + } + } + }, [ + isResolvingExistingAdsAccount, + hasFinishedResolutionForExistingMCAccounts, + ] ); + + return { + isCreatingAccounts: isCreatingAccountsRef.current, + accountCreationResolved: accountCreationResolvedRef.current, + }; +}; + +export default useCreateAccounts; From d1b4c7774caf6fd01fb9a944b9e658b1771a2905 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 24 Sep 2024 00:17:08 +0530 Subject: [PATCH 08/27] Fix: Flicker in component due to resolution state. --- .../connected-google-combo-account-card.js | 20 +++++++++++++------ .../useCreateAccounts.js | 12 +++++++++-- 2 files changed, 24 insertions(+), 8 deletions(-) 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 6ef60d03a2..8cfd132bcc 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 @@ -39,18 +39,26 @@ const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { hasFinishedResolution: hasFinishedResolutionForCurrentAdsAccount, } = useGoogleAdsAccount(); - const { accountCreationResolved, isCreatingAccounts } = useCreateAccounts(); + const { accountsCreated, accountCreationResolved, isCreatingAccounts } = + useCreateAccounts(); if ( - ! hasFinishedResolutionForCurrentMCAccount || - ! hasFinishedResolutionForCurrentAdsAccount || - ! accountCreationResolved + ! accountsCreated && + ( ! hasFinishedResolutionForCurrentAdsAccount || + ! hasFinishedResolutionForCurrentMCAccount || + ! accountCreationResolved ) ) { return } />; } + const creatingAccounts = + isCreatingAccounts || + ( accountsCreated && + ( ! hasFinishedResolutionForCurrentMCAccount || + ! hasFinishedResolutionForCurrentAdsAccount ) ); + const Description = () => { - if ( isCreatingAccounts ) { + if ( creatingAccounts ) { return createInterpolateElement( __( '

You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you.

', @@ -100,7 +108,7 @@ const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { p:

, } ) } - indicator={ isCreatingAccounts ? 'Creating...' : null } + indicator={ creatingAccounts ? 'Creating...' : null } /> ); }; diff --git a/js/src/components/google-combo-account-card/useCreateAccounts.js b/js/src/components/google-combo-account-card/useCreateAccounts.js index 341e57ec6f..bd50116b2f 100644 --- a/js/src/components/google-combo-account-card/useCreateAccounts.js +++ b/js/src/components/google-combo-account-card/useCreateAccounts.js @@ -21,6 +21,7 @@ import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; const useCreateAccounts = () => { const accountCreationResolvedRef = useRef( false ); const isCreatingAccountsRef = useRef( false ); + const accountsCreatedRef = useRef( false ); const { googleAdsAccount, @@ -49,6 +50,7 @@ const useCreateAccounts = () => { receiveMCAccount( account ); invalidateResolution( 'getExistingGoogleAdsAccounts' ); isCreatingAccountsRef.current = false; + accountsCreatedRef.current = true; } }, [ response, loading ] ); @@ -58,10 +60,14 @@ const useCreateAccounts = () => { hasFinishedResolutionForExistingMCAccounts && isCreatingAccountsRef.current === false && accountCreationResolvedRef.current === false && - account === undefined + account === undefined && + ! googleAdsAccount && + ! hasFinishedResolutionForExistingAdsccounts ) { accountCreationResolvedRef.current = true; - const hasExistingAccounts = true; + const hasExistingAccounts = + existingMCAccounts?.length > 0 && + existingAdsAccount?.length > 0; if ( ! hasExistingAccounts ) { const createAccounts = async () => { @@ -70,6 +76,7 @@ const useCreateAccounts = () => { }; isCreatingAccountsRef.current = true; + accountsCreatedRef.current = false; createAccounts(); } } @@ -81,6 +88,7 @@ const useCreateAccounts = () => { return { isCreatingAccounts: isCreatingAccountsRef.current, accountCreationResolved: accountCreationResolvedRef.current, + accountsCreated: accountsCreatedRef.current, }; }; From b9977132234e7843231a621b33df2295fb25d9f3 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 24 Sep 2024 18:53:27 +0530 Subject: [PATCH 09/27] Update account creation checks. --- .../useCreateAccounts.js | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/js/src/components/google-combo-account-card/useCreateAccounts.js b/js/src/components/google-combo-account-card/useCreateAccounts.js index bd50116b2f..db1eaefd5e 100644 --- a/js/src/components/google-combo-account-card/useCreateAccounts.js +++ b/js/src/components/google-combo-account-card/useCreateAccounts.js @@ -12,7 +12,6 @@ import useUpsertAdsAccount from '.~/hooks/useUpsertAdsAccount'; import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts'; import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; import { receiveMCAccount } from '.~/data/actions'; -import { useAppDispatch } from '.~/data'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; /** @@ -25,7 +24,8 @@ const useCreateAccounts = () => { const { googleAdsAccount, - hasFinishedResolution: hasFinishedResolutionForExistingAdsccounts, + hasFinishedResolution: hasFinishedResolutionForExistingAdsccount, + refetchGoogleAdsAccount, } = useGoogleAdsAccount(); const { @@ -42,29 +42,31 @@ const useCreateAccounts = () => { useCreateMCAccount(); const [ upsertAdsAccount, { loading } ] = useUpsertAdsAccount(); - const { invalidateResolution } = useAppDispatch(); // Process account creation completion. useEffect( () => { if ( response?.status === 200 && ! loading ) { receiveMCAccount( account ); - invalidateResolution( 'getExistingGoogleAdsAccounts' ); + refetchGoogleAdsAccount(); isCreatingAccountsRef.current = false; accountsCreatedRef.current = true; } }, [ response, loading ] ); useEffect( () => { - if ( + const existingAccountsResolved = ! isResolvingExistingAdsAccount && - hasFinishedResolutionForExistingMCAccounts && + hasFinishedResolutionForExistingMCAccounts; + + accountCreationResolvedRef.current = existingAccountsResolved; + + if ( + existingAccountsResolved && isCreatingAccountsRef.current === false && - accountCreationResolvedRef.current === false && account === undefined && - ! googleAdsAccount && - ! hasFinishedResolutionForExistingAdsccounts + hasFinishedResolutionForExistingAdsccount && + googleAdsAccount.id === 0 ) { - accountCreationResolvedRef.current = true; const hasExistingAccounts = existingMCAccounts?.length > 0 && existingAdsAccount?.length > 0; From d293f15beae10e6049bbc25bebf46d3ec7e41be6 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Wed, 25 Sep 2024 13:09:25 +0530 Subject: [PATCH 10/27] Update name for resolved checks property. --- .../connected-google-combo-account-card.js | 9 ++++++--- .../google-combo-account-card/useCreateAccounts.js | 10 +++------- 2 files changed, 9 insertions(+), 10 deletions(-) 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 8cfd132bcc..3332ee83d7 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 @@ -39,14 +39,17 @@ const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { hasFinishedResolution: hasFinishedResolutionForCurrentAdsAccount, } = useGoogleAdsAccount(); - const { accountsCreated, accountCreationResolved, isCreatingAccounts } = - useCreateAccounts(); + const { + accountsCreated, + accountCreationChecksResolved, + isCreatingAccounts, + } = useCreateAccounts(); if ( ! accountsCreated && ( ! hasFinishedResolutionForCurrentAdsAccount || ! hasFinishedResolutionForCurrentMCAccount || - ! accountCreationResolved ) + ! accountCreationChecksResolved ) ) { return } />; } diff --git a/js/src/components/google-combo-account-card/useCreateAccounts.js b/js/src/components/google-combo-account-card/useCreateAccounts.js index db1eaefd5e..d58cb93eb8 100644 --- a/js/src/components/google-combo-account-card/useCreateAccounts.js +++ b/js/src/components/google-combo-account-card/useCreateAccounts.js @@ -11,21 +11,19 @@ import useCreateMCAccount from '../google-mc-account-card/useCreateMCAccount'; import useUpsertAdsAccount from '.~/hooks/useUpsertAdsAccount'; import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts'; import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; -import { receiveMCAccount } from '.~/data/actions'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; /** * Custom hook to handle the creation of MC and Ads accounts. */ const useCreateAccounts = () => { - const accountCreationResolvedRef = useRef( false ); + const accountCreationChecksResolvedRef = useRef( false ); const isCreatingAccountsRef = useRef( false ); const accountsCreatedRef = useRef( false ); const { googleAdsAccount, hasFinishedResolution: hasFinishedResolutionForExistingAdsccount, - refetchGoogleAdsAccount, } = useGoogleAdsAccount(); const { @@ -46,8 +44,6 @@ const useCreateAccounts = () => { // Process account creation completion. useEffect( () => { if ( response?.status === 200 && ! loading ) { - receiveMCAccount( account ); - refetchGoogleAdsAccount(); isCreatingAccountsRef.current = false; accountsCreatedRef.current = true; } @@ -58,7 +54,7 @@ const useCreateAccounts = () => { ! isResolvingExistingAdsAccount && hasFinishedResolutionForExistingMCAccounts; - accountCreationResolvedRef.current = existingAccountsResolved; + accountCreationChecksResolvedRef.current = existingAccountsResolved; if ( existingAccountsResolved && @@ -89,7 +85,7 @@ const useCreateAccounts = () => { return { isCreatingAccounts: isCreatingAccountsRef.current, - accountCreationResolved: accountCreationResolvedRef.current, + accountCreationChecksResolved: accountCreationChecksResolvedRef.current, accountsCreated: accountsCreatedRef.current, }; }; From 9ef6c13928e12587acba13e6a5276058d97e2aa4 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Wed, 25 Sep 2024 16:54:48 +0530 Subject: [PATCH 11/27] E2E tests. --- .../specs/setup-mc/step-1-accounts.test.js | 62 +++++++++++++++++++ tests/e2e/utils/mock-requests.js | 46 ++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/tests/e2e/specs/setup-mc/step-1-accounts.test.js b/tests/e2e/specs/setup-mc/step-1-accounts.test.js index 1e8b0cf552..de4c235120 100644 --- a/tests/e2e/specs/setup-mc/step-1-accounts.test.js +++ b/tests/e2e/specs/setup-mc/step-1-accounts.test.js @@ -218,6 +218,68 @@ test.describe( 'Set up accounts', () => { expect( page.url() ).toMatch( baseURL + 'google_auth' ); } ); + + test( 'should create merchant center and ads account if does not exist for the user', async () => { + await setUpAccountsPage.mockJetpackConnected(); + await setUpAccountsPage.mockGoogleConnected(); + + await setupAdsAccountPage.fulfillAdsAccountsRequests( [ + { + id: 0, + currency: 'USD', + status: 'disconnected', + symbol: '$', + }, + { + id: 119119119, + currency: 'USD', + status: 'disconnected', + symbol: '$', + }, + ] ); + + await setupAdsAccountPage.fulfillMCAccountsRequests( [ + [], + { + id: 5432178, + name: null, + subaccount: null, + domain: null, + }, + ] ); + + await setUpAccountsPage.goto(); + const googleAccountCard = setUpAccountsPage.getGoogleAccountCard(); + + await expect( + googleAccountCard.getByText( + /You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you./, + { + exact: true, + } + ) + ).toBeVisible(); + } ); + + test( 'should see the merchant center id and ads account id if connected', async () => { + await setUpAccountsPage.mockGoogleConnected(); + await setUpAccountsPage.mockAdsAccountConnected(); + await setUpAccountsPage.mockMCConnected(); + await setUpAccountsPage.goto(); + + const googleAccountCard = setUpAccountsPage.getGoogleAccountCard(); + await expect( + googleAccountCard.getByText( 'Merchant Center ID: 1234', { + exact: true, + } ) + ).toBeVisible(); + + await expect( + googleAccountCard.getByText( 'Google Ads ID: 12345', { + exact: true, + } ) + ).toBeVisible(); + } ); } ); test.describe( 'Continue button', () => { diff --git a/tests/e2e/utils/mock-requests.js b/tests/e2e/utils/mock-requests.js index 050da0bc4e..a94caebe59 100644 --- a/tests/e2e/utils/mock-requests.js +++ b/tests/e2e/utils/mock-requests.js @@ -38,6 +38,32 @@ export default class MockRequests { } ); } + /** + * Send the different response for each time the route is called. + * + * @param {RegExp|string} url The url to fulfill. + * @param {Array} payloads The payload to send. + * @param {number} status The HTTP status in the response. + * @return {Promise} + */ + async fulfillRequests( url, payloads = [], status = 200 ) { + let callCount = 0; + await this.page.route( url, ( route ) => { + if ( callCount < payloads.length ) { + route.fulfill( { + status, + contentType: 'application/json', + headers: { 'Access-Control-Allow-Origin': '*' }, + body: JSON.stringify( payloads[ callCount ] ), + } ); + } else { + // Optionally handle additional calls beyond the responses array + route.continue(); + } + callCount += 1; + } ); + } + /** * Fulfill the WC options default country request. * @@ -110,6 +136,16 @@ export default class MockRequests { ); } + /** + * Fulfill the MC accounts requests. + * + * @param {Object} payloads + * @return {Promise} + */ + async fulfillMCAccountsRequests( payloads ) { + await this.fulfillRequests( /\/wc\/gla\/mc\/accounts\b/, payloads ); + } + /** * Fulfill the MC accounts claim-overwrite request. * @@ -222,6 +258,16 @@ export default class MockRequests { await this.fulfillRequest( /\/wc\/gla\/ads\/accounts\b/, payload ); } + /** + * Fulfill the Ads Account requests. + * + * @param {Object} payloads + * @return {Promise} + */ + async fulfillAdsAccountsRequests( payloads ) { + await this.fulfillRequests( /\/wc\/gla\/ads\/accounts\b/, payloads ); + } + /** * Fulfill the Ads Account Status request. * From 767a9066c40f18b8c448c20c6d29f0afb4dab0f5 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Wed, 25 Sep 2024 17:30:28 +0530 Subject: [PATCH 12/27] Move hook to hooks directory. --- .../connected-google-combo-account-card.js | 2 +- .../google-combo-account-card => hooks}/useCreateAccounts.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename js/src/{components/google-combo-account-card => hooks}/useCreateAccounts.js (94%) 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 3332ee83d7..8cb5fcf9e9 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 @@ -11,7 +11,7 @@ import AccountCard, { APPEARANCE } from '../account-card'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import useGoogleMCAccount from '.~/hooks/useGoogleMCAccount'; import AppSpinner from '../app-spinner'; -import useCreateAccounts from './useCreateAccounts'; +import useCreateAccounts from '../../hooks/useCreateAccounts'; /** * Clicking on the "connect to a different Google account" button. diff --git a/js/src/components/google-combo-account-card/useCreateAccounts.js b/js/src/hooks/useCreateAccounts.js similarity index 94% rename from js/src/components/google-combo-account-card/useCreateAccounts.js rename to js/src/hooks/useCreateAccounts.js index d58cb93eb8..3e7a71d113 100644 --- a/js/src/components/google-combo-account-card/useCreateAccounts.js +++ b/js/src/hooks/useCreateAccounts.js @@ -7,7 +7,7 @@ import { useEffect, useRef } from '@wordpress/element'; /** * Internal dependencies */ -import useCreateMCAccount from '../google-mc-account-card/useCreateMCAccount'; +import useCreateMCAccount from '../components/google-mc-account-card/useCreateMCAccount'; import useUpsertAdsAccount from '.~/hooks/useUpsertAdsAccount'; import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts'; import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; @@ -54,7 +54,7 @@ const useCreateAccounts = () => { ! isResolvingExistingAdsAccount && hasFinishedResolutionForExistingMCAccounts; - accountCreationChecksResolvedRef.current = existingAccountsResolved; + accountCreationChecksResolvedRef.current = existingAccountsResolved; if ( existingAccountsResolved && From 2de300a5ff953eb4013067fbae43ba7f8dd08676 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Wed, 25 Sep 2024 18:48:02 +0530 Subject: [PATCH 13/27] Add JS tests for useCreateAccounts hook. --- js/src/hooks/useCreateAccounts.js | 7 ++ js/src/hooks/useCreateAccounts.test.js | 105 +++++++++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 js/src/hooks/useCreateAccounts.test.js diff --git a/js/src/hooks/useCreateAccounts.js b/js/src/hooks/useCreateAccounts.js index 3e7a71d113..272dc69a92 100644 --- a/js/src/hooks/useCreateAccounts.js +++ b/js/src/hooks/useCreateAccounts.js @@ -17,6 +17,13 @@ import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; * Custom hook to handle the creation of MC and Ads accounts. */ const useCreateAccounts = () => { + /** + * Refs are used to avoid the re-render of the parent component. + * + * accountCreationChecksResolvedRef - Indicates if the account creation checks have been resolved. + * isCreatingAccountsRef - Indicates if the accounts are being created. + * accountsCreatedRef - Indicates if the accounts have been created. + */ const accountCreationChecksResolvedRef = useRef( false ); const isCreatingAccountsRef = useRef( false ); const accountsCreatedRef = useRef( false ); diff --git a/js/src/hooks/useCreateAccounts.test.js b/js/src/hooks/useCreateAccounts.test.js new file mode 100644 index 0000000000..955e4c46f6 --- /dev/null +++ b/js/src/hooks/useCreateAccounts.test.js @@ -0,0 +1,105 @@ +/* eslint-disable testing-library/no-unnecessary-act */ +/** + * External dependencies + */ +import { renderHook, act } from '@testing-library/react'; + +/** + * Internal dependencies + */ +import useCreateAccounts from './useCreateAccounts'; +import useCreateMCAccount from '../components/google-mc-account-card/useCreateMCAccount'; +import useUpsertAdsAccount from './useUpsertAdsAccount'; +import useExistingGoogleAdsAccounts from './useExistingGoogleAdsAccounts'; +import useExistingGoogleMCAccounts from './useExistingGoogleMCAccounts'; +import useGoogleAdsAccount from './useGoogleAdsAccount'; + +jest.mock( '../components/google-mc-account-card/useCreateMCAccount' ); +jest.mock( './useUpsertAdsAccount' ); +jest.mock( './useExistingGoogleAdsAccounts' ); +jest.mock( './useExistingGoogleMCAccounts' ); +jest.mock( './useGoogleAdsAccount' ); + +describe( 'useCreateAccounts hook', () => { + let handleCreateAccount; + let upsertAdsAccount; + + beforeEach( () => { + handleCreateAccount = jest.fn( () => Promise.resolve() ); + upsertAdsAccount = jest.fn( () => Promise.resolve() ); + + useCreateMCAccount.mockReturnValue( [ + handleCreateAccount, + { data: undefined, response: { status: 200 } }, + ] ); + useUpsertAdsAccount.mockReturnValue( [ + upsertAdsAccount, + { loading: false }, + ] ); + useGoogleAdsAccount.mockReturnValue( { + googleAdsAccount: { id: 0 }, + hasFinishedResolution: true, + } ); + useExistingGoogleMCAccounts.mockReturnValue( { + data: [], + hasFinishedResolution: true, + } ); + useExistingGoogleAdsAccounts.mockReturnValue( { + existingAccounts: [], + isResolving: false, + } ); + } ); + + it( 'should not call "handleCreateAccount" and "upsertAdsAccount" when there are existing accounts', () => { + // Simulate existing accounts + useExistingGoogleMCAccounts.mockReturnValue( { + data: [ { id: 1 } ], + hasFinishedResolution: true, + } ); + useExistingGoogleAdsAccounts.mockReturnValue( { + existingAccounts: [ { id: 1 } ], + isResolving: false, + } ); + + const { result } = renderHook( () => useCreateAccounts() ); + + expect( result.current.isCreatingAccounts ).toBe( false ); + expect( result.current.accountsCreated ).toBe( false ); + expect( handleCreateAccount ).not.toHaveBeenCalled(); + expect( upsertAdsAccount ).not.toHaveBeenCalled(); + } ); + + it( 'should call "handleCreateAccount" and "upsertAdsAccount" when there are no existing accounts', async () => { + // Simulate the initial state and mock behavior for account creation + const { result, rerender } = renderHook( () => useCreateAccounts() ); + + expect( result.current.isCreatingAccounts ).toBe( false ); + + useCreateMCAccount.mockReturnValueOnce( [ + handleCreateAccount, + { data: {}, response: { status: 200 } }, + ] ); + useUpsertAdsAccount.mockReturnValueOnce( [ + upsertAdsAccount, + { loading: false }, + ] ); + + await act( async () => { + rerender(); // Trigger the effect that begins account creation + } ); + + // At this point, isCreatingAccounts should be true + expect( result.current.isCreatingAccounts ).toBe( true ); + + await act( async () => { + rerender(); // Trigger the effect that begins account creation + } ); + + expect( result.current.isCreatingAccounts ).toBe( false ); + expect( result.current.accountsCreated ).toBe( true ); + + // Finally, check that the functions were called correctly + expect( handleCreateAccount ).toHaveBeenCalledTimes( 1 ); + expect( upsertAdsAccount ).toHaveBeenCalledTimes( 1 ); + } ); +} ); From 8239b4764022597de69a4e983d47ce1eff108df0 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Wed, 25 Sep 2024 21:43:14 +0530 Subject: [PATCH 14/27] Add indicator style. --- .../connect-google-combo-account-card.scss | 10 ++++++++++ .../connected-google-combo-account-card.js | 18 +++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss b/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss index 0f29e4ba01..85495cf299 100644 --- a/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss +++ b/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss @@ -9,5 +9,15 @@ .gla-account-card__description { gap: 0; } + + .gla-account-card__indicator { + color: #007CBA; + display: flex; + vertical-align: middle; + + .components-spinner { + margin-top: 0; + } + } } } 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 8cb5fcf9e9..39e1c8cc2e 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 @@ -3,6 +3,7 @@ */ import { createInterpolateElement } from '@wordpress/element'; import { __, sprintf } from '@wordpress/i18n'; +import { Spinner } from '@wordpress/components'; /** * Internal dependencies @@ -97,6 +98,21 @@ const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { ); }; + const Indicator = () => { + if ( creatingAccounts ) { + return ( + <> + + + { __( 'Creating…', 'google-listings-and-ads' ) } + + + ); + } + + return null; + }; + return ( { p:

, } ) } - indicator={ creatingAccounts ? 'Creating...' : null } + indicator={ } /> ); }; From 2e16bf4eb755b0b9a91b10c2ff7827efde39d55b Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Wed, 25 Sep 2024 21:47:51 +0530 Subject: [PATCH 15/27] Fix css lint error. --- .../connect-google-combo-account-card.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss b/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss index 85495cf299..f2dbb97a61 100644 --- a/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss +++ b/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss @@ -11,7 +11,7 @@ } .gla-account-card__indicator { - color: #007CBA; + color: #007cba; display: flex; vertical-align: middle; From 32b47d53dee8da34923552c51f834f4d69a4dcaf Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Fri, 27 Sep 2024 21:17:46 +0530 Subject: [PATCH 16/27] Refactor hook to handle either of the account creation. --- .../account-creation-description.js | 86 +++++++ .../connect-google-combo-account-card.scss | 23 +- .../connected-google-combo-account-card.js | 108 ++++----- ...ounts.js => useAutoCreateAdsMCAccounts.js} | 77 ++++--- .../hooks/useAutoCreateAdsMCAccounts.test.js | 209 ++++++++++++++++++ js/src/hooks/useCreateAccounts.test.js | 105 --------- 6 files changed, 383 insertions(+), 225 deletions(-) create mode 100644 js/src/components/google-combo-account-card/account-creation-description.js rename js/src/hooks/{useCreateAccounts.js => useAutoCreateAdsMCAccounts.js} (54%) create mode 100644 js/src/hooks/useAutoCreateAdsMCAccounts.test.js delete mode 100644 js/src/hooks/useCreateAccounts.test.js diff --git a/js/src/components/google-combo-account-card/account-creation-description.js b/js/src/components/google-combo-account-card/account-creation-description.js new file mode 100644 index 0000000000..ed786b4fa9 --- /dev/null +++ b/js/src/components/google-combo-account-card/account-creation-description.js @@ -0,0 +1,86 @@ +/** + * External dependencies + */ +import { __, sprintf } from '@wordpress/i18n'; + +const AccountCreationDescription = ( { + isCreatingAccounts, + isCreatingMCAccount, + isCreatingAdsAccount, + googleAccount = {}, + googleMCAccount = {}, + googleAdsAccount = {}, +} ) => { + if ( isCreatingAccounts ) { + let description; + + if ( isCreatingMCAccount && isCreatingAdsAccount ) { + description = ( +

+ { __( + 'You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you.', + 'google-listings-and-ads' + ) } +

+ ); + } else if ( isCreatingAdsAccount ) { + description = ( + <> +

+ { __( + 'You don’t have Google Ads account, so we’re creating one for you.', + 'google-listings-and-ads' + ) } +

+ + { __( + 'Required to set up conversion measurement for your store.', + 'google-listings-and-ads' + ) } + + + ); + } else if ( isCreatingMCAccount ) { + description = ( + <> +

+ { __( + 'You don’t have Merchant Center account, so we’re creating one for you.', + 'google-listings-and-ads' + ) } +

+ + { __( + 'Required to sync products so they show on Google.', + 'google-listings-and-ads' + ) } + + + ); + } + + return description; + } + + return ( +
+

{ googleAccount?.email }

+

+ { sprintf( + // Translators: %s is the Merchant Center ID + __( 'Merchant Center ID: %s', 'google-listings-and-ads' ), + googleMCAccount.id ?? 0 + ) } +

+

+ { sprintf( + // Translators: %s is the Google Ads ID + __( 'Google Ads ID: %s', 'google-listings-and-ads' ), + googleAdsAccount.id ?? 0 + ) } +

+
+ ); +}; + +export default AccountCreationDescription; diff --git a/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss b/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss index f2dbb97a61..991927082a 100644 --- a/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss +++ b/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss @@ -1,23 +1,10 @@ -.gla-connect-google-combo-account-card { +.gla-google-combo-account-card--connected { - .gla-account-card__helper { - font-size: $gla-font-base; + .gla-account-card__icon { + align-self: flex-start; } - &.gla-connected-google-combo-account-card { - - .gla-account-card__description { - gap: 0; - } - - .gla-account-card__indicator { - color: #007cba; - display: flex; - vertical-align: middle; - - .components-spinner { - margin-top: 0; - } - } + .gla-account-card__description p { + margin: 0; } } 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 39e1c8cc2e..16a8436030 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 @@ -1,9 +1,7 @@ /** * External dependencies */ -import { createInterpolateElement } from '@wordpress/element'; -import { __, sprintf } from '@wordpress/i18n'; -import { Spinner } from '@wordpress/components'; +import { __ } from '@wordpress/i18n'; /** * Internal dependencies @@ -12,7 +10,9 @@ import AccountCard, { APPEARANCE } from '../account-card'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import useGoogleMCAccount from '.~/hooks/useGoogleMCAccount'; import AppSpinner from '../app-spinner'; -import useCreateAccounts from '../../hooks/useCreateAccounts'; +import useAutoCreateAdsMCAccounts from '../../hooks/useAutoCreateAdsMCAccounts'; +import LoadingLabel from '../loading-label/loading-label'; +import AccountCreationDescription from './account-creation-description'; /** * Clicking on the "connect to a different Google account" button. @@ -43,8 +43,11 @@ const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { const { accountsCreated, accountCreationChecksResolved, - isCreatingAccounts, - } = useCreateAccounts(); + isCreatingAdsAccount, + isCreatingMCAccount, + } = useAutoCreateAdsMCAccounts(); + + const isCreatingAccounts = isCreatingAdsAccount || isCreatingMCAccount; if ( ! accountsCreated && @@ -61,73 +64,38 @@ const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { ( ! hasFinishedResolutionForCurrentMCAccount || ! hasFinishedResolutionForCurrentAdsAccount ) ); - const Description = () => { - if ( creatingAccounts ) { - return createInterpolateElement( - __( - '

You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you.

', - 'google-listings-and-ads' - ), - { - p:

, - } - ); - } - - return ( - <> -

{ googleAccount?.email }

-

- { sprintf( - // Translators: %s is the Merchant Center ID - __( - 'Merchant Center ID: %s', - 'google-listings-and-ads' - ), - googleMCAccount?.id - ) } -

-

- { sprintf( - // Translators: %s is the Google Ads ID - __( 'Google Ads ID: %s', 'google-listings-and-ads' ), - googleAdsAccount?.id - ) } -

- - ); - }; - - const Indicator = () => { - if ( creatingAccounts ) { - return ( - <> - - - { __( 'Creating…', 'google-listings-and-ads' ) } - - - ); - } - - return null; - }; - return ( } - helper={ createInterpolateElement( - __( - '

Merchant Center is required to sync products so they show on Google. Google Ads is required to set up conversion measurement for your store.

', - 'google-listings-and-ads' - ), - { - p:

, - } - ) } - indicator={ } + className="gla-google-combo-account-card--connected" + description={ + + } + helper={ + isCreatingAdsAccount && + isCreatingMCAccount && ( +

+ { __( + 'Merchant Center is required to sync products so they show on Google. Google Ads is required to set up conversion measurement for your store.', + 'google-listings-and-ads' + ) } +

+ ) + } + indicator={ + creatingAccounts ? ( + + ) : null + } /> ); }; diff --git a/js/src/hooks/useCreateAccounts.js b/js/src/hooks/useAutoCreateAdsMCAccounts.js similarity index 54% rename from js/src/hooks/useCreateAccounts.js rename to js/src/hooks/useAutoCreateAdsMCAccounts.js index 272dc69a92..2d09765362 100644 --- a/js/src/hooks/useCreateAccounts.js +++ b/js/src/hooks/useAutoCreateAdsMCAccounts.js @@ -1,4 +1,3 @@ -/* eslint-disable react-hooks/exhaustive-deps */ /** * External dependencies */ @@ -11,12 +10,16 @@ import useCreateMCAccount from '../components/google-mc-account-card/useCreateMC import useUpsertAdsAccount from '.~/hooks/useUpsertAdsAccount'; import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts'; import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; -import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; /** * Custom hook to handle the creation of MC and Ads accounts. + * + * @return {Object} Object with the following properties: + * - isCreatingAccounts: Indicates if the accounts are being created. + * - accountCreationChecksResolved: Indicates if the account creation checks have been resolved. + * - accountsCreated: Indicates if the accounts have been created */ -const useCreateAccounts = () => { +const useAutoCreateAdsMCAccounts = () => { /** * Refs are used to avoid the re-render of the parent component. * @@ -26,13 +29,10 @@ const useCreateAccounts = () => { */ const accountCreationChecksResolvedRef = useRef( false ); const isCreatingAccountsRef = useRef( false ); + const isCreatingAdsAccountsRef = useRef( false ); + const isCreatingMCAccountsRef = useRef( false ); const accountsCreatedRef = useRef( false ); - const { - googleAdsAccount, - hasFinishedResolution: hasFinishedResolutionForExistingAdsccount, - } = useGoogleAdsAccount(); - const { data: existingMCAccounts, hasFinishedResolution: hasFinishedResolutionForExistingMCAccounts, @@ -43,14 +43,35 @@ const useCreateAccounts = () => { isResolving: isResolvingExistingAdsAccount, } = useExistingGoogleAdsAccounts(); - const [ handleCreateAccount, { data: account, response } ] = - useCreateMCAccount(); - + const [ handleCreateAccount, { response } ] = useCreateMCAccount(); const [ upsertAdsAccount, { loading } ] = useUpsertAdsAccount(); - // Process account creation completion. + const createAccounts = async () => { + const hasExistingMCAccount = existingMCAccounts.length > 0; + const hasExistingAdsAccount = existingAdsAccount.length > 0; + + isCreatingMCAccountsRef.current = ! hasExistingMCAccount; + isCreatingAdsAccountsRef.current = ! hasExistingAdsAccount; + isCreatingAccountsRef.current = + ! hasExistingAdsAccount || ! hasExistingMCAccount; + + if ( ! hasExistingMCAccount ) { + await handleCreateAccount(); + } + + if ( ! hasExistingAdsAccount ) { + await upsertAdsAccount(); + } + }; + useEffect( () => { - if ( response?.status === 200 && ! loading ) { + if ( + isCreatingAccountsRef.current === true && + response?.status === 200 && + ! loading + ) { + isCreatingMCAccountsRef.current = false; + isCreatingAdsAccountsRef.current = false; isCreatingAccountsRef.current = false; accountsCreatedRef.current = true; } @@ -65,36 +86,28 @@ const useCreateAccounts = () => { if ( existingAccountsResolved && - isCreatingAccountsRef.current === false && - account === undefined && - hasFinishedResolutionForExistingAdsccount && - googleAdsAccount.id === 0 + isCreatingAccountsRef.current === false ) { - const hasExistingAccounts = - existingMCAccounts?.length > 0 && - existingAdsAccount?.length > 0; - - if ( ! hasExistingAccounts ) { - const createAccounts = async () => { - await handleCreateAccount(); - await upsertAdsAccount(); - }; - - isCreatingAccountsRef.current = true; - accountsCreatedRef.current = false; + const hasExistingMCAccount = existingMCAccounts?.length > 0; + const hasExistingAdsAccount = existingAdsAccount?.length > 0; + + if ( ! hasExistingMCAccount || ! hasExistingAdsAccount ) { createAccounts(); } } + // eslint-disable-next-line react-hooks/exhaustive-deps }, [ isResolvingExistingAdsAccount, hasFinishedResolutionForExistingMCAccounts, ] ); return { - isCreatingAccounts: isCreatingAccountsRef.current, - accountCreationChecksResolved: accountCreationChecksResolvedRef.current, accountsCreated: accountsCreatedRef.current, + accountCreationChecksResolved: accountCreationChecksResolvedRef.current, + isCreatingAccounts: isCreatingAccountsRef.current, + isCreatingAdsAccount: isCreatingAdsAccountsRef.current, + isCreatingMCAccount: isCreatingMCAccountsRef.current, }; }; -export default useCreateAccounts; +export default useAutoCreateAdsMCAccounts; diff --git a/js/src/hooks/useAutoCreateAdsMCAccounts.test.js b/js/src/hooks/useAutoCreateAdsMCAccounts.test.js new file mode 100644 index 0000000000..ccd6c779be --- /dev/null +++ b/js/src/hooks/useAutoCreateAdsMCAccounts.test.js @@ -0,0 +1,209 @@ +/* eslint-disable testing-library/no-unnecessary-act */ +/** + * External dependencies + */ +import { renderHook, act } from '@testing-library/react'; + +/** + * Internal dependencies + */ +import useCreateAccounts from './useAutoCreateAdsMCAccounts'; +import useCreateMCAccount from '../components/google-mc-account-card/useCreateMCAccount'; +import useUpsertAdsAccount from './useUpsertAdsAccount'; +import useExistingGoogleAdsAccounts from './useExistingGoogleAdsAccounts'; +import useExistingGoogleMCAccounts from './useExistingGoogleMCAccounts'; + +jest.mock( '../components/google-mc-account-card/useCreateMCAccount' ); +jest.mock( './useUpsertAdsAccount' ); +jest.mock( './useExistingGoogleAdsAccounts' ); +jest.mock( './useExistingGoogleMCAccounts' ); +jest.mock( './useGoogleAdsAccount' ); + +describe( 'useAutoCreateAdsMCAccounts hook', () => { + let handleCreateAccount; + let upsertAdsAccount; + + beforeEach( () => { + handleCreateAccount = jest.fn( () => Promise.resolve() ); + upsertAdsAccount = jest.fn( () => Promise.resolve() ); + + useCreateMCAccount.mockReturnValue( [ + handleCreateAccount, + { response: { status: 200 } }, + ] ); + useUpsertAdsAccount.mockReturnValue( [ + upsertAdsAccount, + { loading: false }, + ] ); + useExistingGoogleMCAccounts.mockReturnValue( { + data: [], + hasFinishedResolution: true, + } ); + useExistingGoogleAdsAccounts.mockReturnValue( { + existingAccounts: [], + isResolving: false, + } ); + } ); + + it( 'should not call "handleCreateAccount" and "upsertAdsAccount" when there are existing accounts', () => { + // Simulate existing accounts + useExistingGoogleMCAccounts.mockReturnValue( { + data: [ { id: 1 } ], + hasFinishedResolution: true, + } ); + useExistingGoogleAdsAccounts.mockReturnValue( { + existingAccounts: [ { id: 1 } ], + isResolving: false, + } ); + + const { result } = renderHook( () => useCreateAccounts() ); + + expect( result.current.isCreatingAccounts ).toBe( false ); + expect( result.current.accountsCreated ).toBe( false ); + expect( handleCreateAccount ).not.toHaveBeenCalled(); + expect( upsertAdsAccount ).not.toHaveBeenCalled(); + } ); + + it( 'should call "handleCreateAccount" and "upsertAdsAccount" when there are no existing accounts', async () => { + // Simulate the initial state and mock behavior for account creation + const { result, rerender } = renderHook( () => useCreateAccounts() ); + + expect( result.current.isCreatingAccounts ).toBe( false ); + + useCreateMCAccount.mockReturnValueOnce( [ + handleCreateAccount, + { response: { status: 200 } }, + ] ); + useUpsertAdsAccount.mockReturnValueOnce( [ + upsertAdsAccount, + { loading: false }, + ] ); + + await act( async () => { + rerender(); // Trigger the effect that begins account creation + } ); + + // At this point, isCreatingAccounts should be true + expect( result.current.isCreatingAdsAccount ).toBe( true ); + expect( result.current.isCreatingMCAccount ).toBe( true ); + + await act( async () => { + rerender(); + } ); + + expect( result.current.isCreatingAccounts ).toBe( false ); + expect( result.current.accountsCreated ).toBe( true ); + + // Finally, check that the functions were called correctly + expect( handleCreateAccount ).toHaveBeenCalledTimes( 1 ); + expect( upsertAdsAccount ).toHaveBeenCalledTimes( 1 ); + } ); + + it( 'should create Merchant Center account only when there is no existing MC account, but there is an existing Ads account', async () => { + // Simulate no existing MC account but an existing Ads account + useExistingGoogleMCAccounts.mockReturnValue( { + data: [], // No existing MC account + hasFinishedResolution: true, + } ); + useExistingGoogleAdsAccounts.mockReturnValue( { + existingAccounts: [ { id: 1 } ], // Existing Ads account + isResolving: false, + } ); + + // Step 1: Initial render - No response, loading is true + useCreateMCAccount.mockReturnValueOnce( [ + handleCreateAccount, + { response: undefined, loading: true }, // Initially no response, loading is true + ] ); + + const { result, rerender } = renderHook( () => useCreateAccounts() ); + + // Initially, it should not be creating accounts + expect( result.current.isCreatingAccounts ).toBe( false ); + + // Trigger the effect that starts account creation + await act( async () => { + rerender(); // Simulate the effect firing + } ); + + // Step 2: At this point, MC account creation should have started + expect( result.current.isCreatingMCAccount ).toBe( true ); + expect( result.current.isCreatingAdsAccount ).toBe( false ); // Since Ads account exists + expect( handleCreateAccount ).toHaveBeenCalledTimes( 1 ); + expect( upsertAdsAccount ).not.toHaveBeenCalled(); + + // Step 3: Simulate the response for MC account creation + useCreateMCAccount.mockReturnValueOnce( [ + handleCreateAccount, + { response: { status: 200 }, loading: false }, // Now account creation is complete + ] ); + + // Trigger a rerender to simulate the async function resolving + await act( async () => { + rerender(); // Re-render after response has been updated + } ); + + // Step 4: Final assertions after account creation has completed + expect( result.current.isCreatingAccounts ).toBe( false ); // Account creation should be complete + expect( result.current.accountsCreated ).toBe( true ); // The account has been created + expect( result.current.isCreatingMCAccount ).toBe( false ); // MC account creation finished + + // Finally, verify that only handleCreateAccount was called, not upsertAdsAccount + expect( handleCreateAccount ).toHaveBeenCalledTimes( 1 ); + expect( upsertAdsAccount ).not.toHaveBeenCalled(); + } ); + + it( 'should create Ads account only when there is no existing Ads account, but there is an existing Merchant Center account', async () => { + // Simulate existing MC account but no existing Ads account + useExistingGoogleMCAccounts.mockReturnValue( { + data: [ { id: 1 } ], // Existing MC account + hasFinishedResolution: true, + } ); + useExistingGoogleAdsAccounts.mockReturnValue( { + existingAccounts: [], // No existing Ads account + isResolving: false, + } ); + + // Step 1: Initial render - No response, loading is true for Ads account creation + useUpsertAdsAccount.mockReturnValueOnce( [ + upsertAdsAccount, + { response: undefined, loading: true }, // Initially no response, loading is true for Ads account creation + ] ); + + const { result, rerender } = renderHook( () => useCreateAccounts() ); + + // Initially, it should not be creating accounts + expect( result.current.isCreatingAccounts ).toBe( false ); + + // Trigger the effect that starts account creation + await act( async () => { + rerender(); // Simulate the effect firing + } ); + + // Step 2: At this point, Ads account creation should have started + expect( result.current.isCreatingAdsAccount ).toBe( true ); + expect( result.current.isCreatingMCAccount ).toBe( false ); // Since MC account exists + expect( upsertAdsAccount ).toHaveBeenCalledTimes( 1 ); + expect( handleCreateAccount ).not.toHaveBeenCalled(); // MC account creation shouldn't be triggered + + // Step 3: Simulate the response for Ads account creation + useUpsertAdsAccount.mockReturnValueOnce( [ + upsertAdsAccount, + { response: { status: 200 }, loading: false }, // Now Ads account creation is complete + ] ); + + // Trigger a rerender to simulate the async function resolving + await act( async () => { + rerender(); // Re-render after response has been updated + } ); + + // Step 4: Final assertions after Ads account creation has completed + expect( result.current.isCreatingAccounts ).toBe( false ); // Account creation should be complete + expect( result.current.accountsCreated ).toBe( true ); // The account has been created + expect( result.current.isCreatingAdsAccount ).toBe( false ); // Ads account creation finished + + // Finally, verify that only upsertAdsAccount was called, not handleCreateAccount + expect( upsertAdsAccount ).toHaveBeenCalledTimes( 1 ); + expect( handleCreateAccount ).not.toHaveBeenCalled(); + } ); +} ); diff --git a/js/src/hooks/useCreateAccounts.test.js b/js/src/hooks/useCreateAccounts.test.js deleted file mode 100644 index 955e4c46f6..0000000000 --- a/js/src/hooks/useCreateAccounts.test.js +++ /dev/null @@ -1,105 +0,0 @@ -/* eslint-disable testing-library/no-unnecessary-act */ -/** - * External dependencies - */ -import { renderHook, act } from '@testing-library/react'; - -/** - * Internal dependencies - */ -import useCreateAccounts from './useCreateAccounts'; -import useCreateMCAccount from '../components/google-mc-account-card/useCreateMCAccount'; -import useUpsertAdsAccount from './useUpsertAdsAccount'; -import useExistingGoogleAdsAccounts from './useExistingGoogleAdsAccounts'; -import useExistingGoogleMCAccounts from './useExistingGoogleMCAccounts'; -import useGoogleAdsAccount from './useGoogleAdsAccount'; - -jest.mock( '../components/google-mc-account-card/useCreateMCAccount' ); -jest.mock( './useUpsertAdsAccount' ); -jest.mock( './useExistingGoogleAdsAccounts' ); -jest.mock( './useExistingGoogleMCAccounts' ); -jest.mock( './useGoogleAdsAccount' ); - -describe( 'useCreateAccounts hook', () => { - let handleCreateAccount; - let upsertAdsAccount; - - beforeEach( () => { - handleCreateAccount = jest.fn( () => Promise.resolve() ); - upsertAdsAccount = jest.fn( () => Promise.resolve() ); - - useCreateMCAccount.mockReturnValue( [ - handleCreateAccount, - { data: undefined, response: { status: 200 } }, - ] ); - useUpsertAdsAccount.mockReturnValue( [ - upsertAdsAccount, - { loading: false }, - ] ); - useGoogleAdsAccount.mockReturnValue( { - googleAdsAccount: { id: 0 }, - hasFinishedResolution: true, - } ); - useExistingGoogleMCAccounts.mockReturnValue( { - data: [], - hasFinishedResolution: true, - } ); - useExistingGoogleAdsAccounts.mockReturnValue( { - existingAccounts: [], - isResolving: false, - } ); - } ); - - it( 'should not call "handleCreateAccount" and "upsertAdsAccount" when there are existing accounts', () => { - // Simulate existing accounts - useExistingGoogleMCAccounts.mockReturnValue( { - data: [ { id: 1 } ], - hasFinishedResolution: true, - } ); - useExistingGoogleAdsAccounts.mockReturnValue( { - existingAccounts: [ { id: 1 } ], - isResolving: false, - } ); - - const { result } = renderHook( () => useCreateAccounts() ); - - expect( result.current.isCreatingAccounts ).toBe( false ); - expect( result.current.accountsCreated ).toBe( false ); - expect( handleCreateAccount ).not.toHaveBeenCalled(); - expect( upsertAdsAccount ).not.toHaveBeenCalled(); - } ); - - it( 'should call "handleCreateAccount" and "upsertAdsAccount" when there are no existing accounts', async () => { - // Simulate the initial state and mock behavior for account creation - const { result, rerender } = renderHook( () => useCreateAccounts() ); - - expect( result.current.isCreatingAccounts ).toBe( false ); - - useCreateMCAccount.mockReturnValueOnce( [ - handleCreateAccount, - { data: {}, response: { status: 200 } }, - ] ); - useUpsertAdsAccount.mockReturnValueOnce( [ - upsertAdsAccount, - { loading: false }, - ] ); - - await act( async () => { - rerender(); // Trigger the effect that begins account creation - } ); - - // At this point, isCreatingAccounts should be true - expect( result.current.isCreatingAccounts ).toBe( true ); - - await act( async () => { - rerender(); // Trigger the effect that begins account creation - } ); - - expect( result.current.isCreatingAccounts ).toBe( false ); - expect( result.current.accountsCreated ).toBe( true ); - - // Finally, check that the functions were called correctly - expect( handleCreateAccount ).toHaveBeenCalledTimes( 1 ); - expect( upsertAdsAccount ).toHaveBeenCalledTimes( 1 ); - } ); -} ); From f55fd1dcb78a8d2b29851e0e9ef568926892d52d Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Mon, 30 Sep 2024 19:28:03 +0530 Subject: [PATCH 17/27] CR feedback - round 2. --- .../account-creation-description.js | 138 +++++++++++------- .../connected-google-combo-account-card.js | 45 +++--- js/src/hooks/useAutoCreateAdsMCAccounts.js | 14 +- .../hooks/useAutoCreateAdsMCAccounts.test.js | 21 +-- 4 files changed, 132 insertions(+), 86 deletions(-) diff --git a/js/src/components/google-combo-account-card/account-creation-description.js b/js/src/components/google-combo-account-card/account-creation-description.js index ed786b4fa9..7a3a96c6cf 100644 --- a/js/src/components/google-combo-account-card/account-creation-description.js +++ b/js/src/components/google-combo-account-card/account-creation-description.js @@ -3,82 +3,116 @@ */ import { __, sprintf } from '@wordpress/i18n'; +/** + * Internal dependencies + */ +import useGoogleAccount from '.~/hooks/useGoogleAccount'; + +/** + * Renders the description for the account creation card. + * + * @param {*} param0 Props. + * @param {boolean} param0.isCreatingAccounts Whether accounts are being created. + * @param {boolean} param0.isCreatingMCAccount Whether Merchant Center account is being created. + * @param {boolean} param0.isCreatingAdsAccount Whether Google Ads account is being created. + * @param {Object} param0.googleMCAccount Google Merchant Center account. + * @param {Object} param0.googleAdsAccount Google Ads account. + */ const AccountCreationDescription = ( { isCreatingAccounts, isCreatingMCAccount, isCreatingAdsAccount, - googleAccount = {}, googleMCAccount = {}, googleAdsAccount = {}, } ) => { - if ( isCreatingAccounts ) { + const { google } = useGoogleAccount(); + + const getDescription = () => { let description; - if ( isCreatingMCAccount && isCreatingAdsAccount ) { - description = ( -

- { __( - 'You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you.', - 'google-listings-and-ads' - ) } -

- ); - } else if ( isCreatingAdsAccount ) { - description = ( - <> + if ( isCreatingAccounts ) { + if ( isCreatingMCAccount && isCreatingAdsAccount ) { + description = (

{ __( - 'You don’t have Google Ads account, so we’re creating one for you.', + 'You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you.', 'google-listings-and-ads' ) }

- - { __( - 'Required to set up conversion measurement for your store.', - 'google-listings-and-ads' - ) } - - - ); - } else if ( isCreatingMCAccount ) { + ); + } else if ( isCreatingAdsAccount ) { + description = ( + <> +

+ { __( + 'You don’t have Google Ads account, so we’re creating one for you.', + 'google-listings-and-ads' + ) } +

+ + { __( + 'Required to set up conversion measurement for your store.', + 'google-listings-and-ads' + ) } + + + ); + } else if ( isCreatingMCAccount ) { + description = ( + <> +

+ { __( + 'You don’t have Merchant Center account, so we’re creating one for you.', + 'google-listings-and-ads' + ) } +

+ + { __( + 'Required to sync products so they show on Google.', + 'google-listings-and-ads' + ) } + + + ); + } + } else { description = ( <> -

- { __( - 'You don’t have Merchant Center account, so we’re creating one for you.', - 'google-listings-and-ads' - ) } -

- - { __( - 'Required to sync products so they show on Google.', - 'google-listings-and-ads' - ) } - +

{ google?.email }

+ { googleMCAccount?.id && ( +

+ { sprintf( + // Translators: %s is the Merchant Center ID + __( + 'Merchant Center ID: %s', + 'google-listings-and-ads' + ), + googleMCAccount.id + ) } +

+ ) } + { googleAdsAccount?.id && ( +

+ { sprintf( + // Translators: %s is the Google Ads ID + __( + 'Google Ads ID: %s', + 'google-listings-and-ads' + ), + googleAdsAccount.id + ) } +

+ ) } ); } return description; - } + }; return (
-

{ googleAccount?.email }

-

- { sprintf( - // Translators: %s is the Merchant Center ID - __( 'Merchant Center ID: %s', 'google-listings-and-ads' ), - googleMCAccount.id ?? 0 - ) } -

-

- { sprintf( - // Translators: %s is the Google Ads ID - __( 'Google Ads ID: %s', 'google-listings-and-ads' ), - googleAdsAccount.id ?? 0 - ) } -

+ { getDescription() }
); }; 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 16a8436030..618549ac3c 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 @@ -64,13 +64,36 @@ const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { ( ! hasFinishedResolutionForCurrentMCAccount || ! hasFinishedResolutionForCurrentAdsAccount ) ); + const getHelper = () => + isCreatingAdsAccount && + isCreatingMCAccount && ( +

+ { __( + 'Merchant Center is required to sync products so they show on Google. Google Ads is required to set up conversion measurement for your store.', + 'google-listings-and-ads' + ) } +

+ ); + + const getIndicator = () => { + if ( creatingAccounts ) { + return ( + + ); + } + + return null; + }; + return ( { googleAdsAccount={ googleAdsAccount } /> } - helper={ - isCreatingAdsAccount && - isCreatingMCAccount && ( -

- { __( - 'Merchant Center is required to sync products so they show on Google. Google Ads is required to set up conversion measurement for your store.', - 'google-listings-and-ads' - ) } -

- ) - } - indicator={ - creatingAccounts ? ( - - ) : null - } + helper={ getHelper() } + indicator={ getIndicator() } /> ); }; diff --git a/js/src/hooks/useAutoCreateAdsMCAccounts.js b/js/src/hooks/useAutoCreateAdsMCAccounts.js index 2d09765362..d5e21de5c7 100644 --- a/js/src/hooks/useAutoCreateAdsMCAccounts.js +++ b/js/src/hooks/useAutoCreateAdsMCAccounts.js @@ -12,12 +12,15 @@ import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts' import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; /** - * Custom hook to handle the creation of MC and Ads accounts. + * Custom hook to handle the creation of Google Merchant Center (MC) and Google Ads accounts. * - * @return {Object} Object with the following properties: - * - isCreatingAccounts: Indicates if the accounts are being created. - * - accountCreationChecksResolved: Indicates if the account creation checks have been resolved. - * - accountsCreated: Indicates if the accounts have been created + * @typedef {Object} AutoCreateAccountsStatus + * @property {boolean} isCreatingAdsAccount Indicates if the Google Ads account is currently being created. + * @property {boolean} isCreatingMCAccount Indicates if the Google Merchant Center account is currently being created. + * @property {boolean} accountCreationChecksResolved Indicates if the account creation checks (for existing accounts) have been resolved. + * @property {boolean} accountsCreated Indicates if both the Google Ads and Google Merchant Center accounts have been successfully created. + * + * @return {AutoCreateAccountsStatus} Object containing properties related to the account creation status. */ const useAutoCreateAdsMCAccounts = () => { /** @@ -104,7 +107,6 @@ const useAutoCreateAdsMCAccounts = () => { return { accountsCreated: accountsCreatedRef.current, accountCreationChecksResolved: accountCreationChecksResolvedRef.current, - isCreatingAccounts: isCreatingAccountsRef.current, isCreatingAdsAccount: isCreatingAdsAccountsRef.current, isCreatingMCAccount: isCreatingMCAccountsRef.current, }; diff --git a/js/src/hooks/useAutoCreateAdsMCAccounts.test.js b/js/src/hooks/useAutoCreateAdsMCAccounts.test.js index ccd6c779be..844b968ae8 100644 --- a/js/src/hooks/useAutoCreateAdsMCAccounts.test.js +++ b/js/src/hooks/useAutoCreateAdsMCAccounts.test.js @@ -58,7 +58,8 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { const { result } = renderHook( () => useCreateAccounts() ); - expect( result.current.isCreatingAccounts ).toBe( false ); + expect( result.current.isCreatingAdsAccount ).toBe( false ); + expect( result.current.isCreatingMCAccount ).toBe( false ); expect( result.current.accountsCreated ).toBe( false ); expect( handleCreateAccount ).not.toHaveBeenCalled(); expect( upsertAdsAccount ).not.toHaveBeenCalled(); @@ -68,7 +69,8 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { // Simulate the initial state and mock behavior for account creation const { result, rerender } = renderHook( () => useCreateAccounts() ); - expect( result.current.isCreatingAccounts ).toBe( false ); + expect( result.current.isCreatingAdsAccount ).toBe( false ); + expect( result.current.isCreatingMCAccount ).toBe( false ); useCreateMCAccount.mockReturnValueOnce( [ handleCreateAccount, @@ -83,7 +85,6 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { rerender(); // Trigger the effect that begins account creation } ); - // At this point, isCreatingAccounts should be true expect( result.current.isCreatingAdsAccount ).toBe( true ); expect( result.current.isCreatingMCAccount ).toBe( true ); @@ -91,7 +92,8 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { rerender(); } ); - expect( result.current.isCreatingAccounts ).toBe( false ); + expect( result.current.isCreatingAdsAccount ).toBe( false ); + expect( result.current.isCreatingMCAccount ).toBe( false ); expect( result.current.accountsCreated ).toBe( true ); // Finally, check that the functions were called correctly @@ -119,7 +121,8 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { const { result, rerender } = renderHook( () => useCreateAccounts() ); // Initially, it should not be creating accounts - expect( result.current.isCreatingAccounts ).toBe( false ); + expect( result.current.isCreatingAdsAccount ).toBe( false ); + expect( result.current.isCreatingMCAccount ).toBe( false ); // Trigger the effect that starts account creation await act( async () => { @@ -144,9 +147,9 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { } ); // Step 4: Final assertions after account creation has completed - expect( result.current.isCreatingAccounts ).toBe( false ); // Account creation should be complete + expect( result.current.isCreatingAdsAccount ).toBe( false ); + expect( result.current.isCreatingMCAccount ).toBe( false ); expect( result.current.accountsCreated ).toBe( true ); // The account has been created - expect( result.current.isCreatingMCAccount ).toBe( false ); // MC account creation finished // Finally, verify that only handleCreateAccount was called, not upsertAdsAccount expect( handleCreateAccount ).toHaveBeenCalledTimes( 1 ); @@ -173,7 +176,8 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { const { result, rerender } = renderHook( () => useCreateAccounts() ); // Initially, it should not be creating accounts - expect( result.current.isCreatingAccounts ).toBe( false ); + expect( result.current.isCreatingAdsAccount ).toBe( false ); + expect( result.current.isCreatingMCAccount ).toBe( false ); // Trigger the effect that starts account creation await act( async () => { @@ -198,7 +202,6 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { } ); // Step 4: Final assertions after Ads account creation has completed - expect( result.current.isCreatingAccounts ).toBe( false ); // Account creation should be complete expect( result.current.accountsCreated ).toBe( true ); // The account has been created expect( result.current.isCreatingAdsAccount ).toBe( false ); // Ads account creation finished From 95fe0cb4831f9076799a3833a84fc398f0764c62 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 1 Oct 2024 23:50:15 +0530 Subject: [PATCH 18/27] Add CR feedback changes. --- .../account-creation-description.js | 78 +++++++++---------- .../connected-google-combo-account-card.js | 1 - js/src/hooks/useAutoCreateAdsMCAccounts.js | 6 +- js/src/hooks/useExistingGoogleAdsAccounts.js | 5 +- 4 files changed, 45 insertions(+), 45 deletions(-) diff --git a/js/src/components/google-combo-account-card/account-creation-description.js b/js/src/components/google-combo-account-card/account-creation-description.js index 7a3a96c6cf..78c75005d9 100644 --- a/js/src/components/google-combo-account-card/account-creation-description.js +++ b/js/src/components/google-combo-account-card/account-creation-description.js @@ -11,12 +11,12 @@ import useGoogleAccount from '.~/hooks/useGoogleAccount'; /** * Renders the description for the account creation card. * - * @param {*} param0 Props. - * @param {boolean} param0.isCreatingAccounts Whether accounts are being created. - * @param {boolean} param0.isCreatingMCAccount Whether Merchant Center account is being created. - * @param {boolean} param0.isCreatingAdsAccount Whether Google Ads account is being created. - * @param {Object} param0.googleMCAccount Google Merchant Center account. - * @param {Object} param0.googleAdsAccount Google Ads account. + * @param {Object} props Props. + * @param {boolean} props.isCreatingAccounts Whether accounts are being created. + * @param {boolean} props.isCreatingMCAccount Whether Merchant Center account is being created. + * @param {boolean} props.isCreatingAdsAccount Whether Google Ads account is being created. + * @param {Object} props.googleMCAccount Google Merchant Center account. + * @param {Object} props.googleAdsAccount Google Ads account. */ const AccountCreationDescription = ( { isCreatingAccounts, @@ -32,7 +32,7 @@ const AccountCreationDescription = ( { if ( isCreatingAccounts ) { if ( isCreatingMCAccount && isCreatingAdsAccount ) { - description = ( + return (

{ __( 'You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you.', @@ -41,7 +41,7 @@ const AccountCreationDescription = ( {

); } else if ( isCreatingAdsAccount ) { - description = ( + return ( <>

{ __( @@ -58,7 +58,7 @@ const AccountCreationDescription = ( { ); } else if ( isCreatingMCAccount ) { - description = ( + return ( <>

{ __( @@ -75,39 +75,37 @@ const AccountCreationDescription = ( { ); } - } else { - description = ( - <> -

{ google?.email }

- { googleMCAccount?.id && ( -

- { sprintf( - // Translators: %s is the Merchant Center ID - __( - 'Merchant Center ID: %s', - 'google-listings-and-ads' - ), - googleMCAccount.id - ) } -

- ) } - { googleAdsAccount?.id && ( -

- { sprintf( - // Translators: %s is the Google Ads ID - __( - 'Google Ads ID: %s', - 'google-listings-and-ads' - ), - googleAdsAccount.id - ) } -

- ) } - - ); } - return description; + return ( + <> +

{ google?.email }

+ { googleMCAccount?.id && ( +

+ { sprintf( + // Translators: %s is the Merchant Center ID + __( + 'Merchant Center ID: %s', + 'google-listings-and-ads' + ), + googleMCAccount.id + ) } +

+ ) } + { googleAdsAccount?.id && ( +

+ { sprintf( + // Translators: %s is the Google Ads ID + __( + 'Google Ads ID: %s', + 'google-listings-and-ads' + ), + googleAdsAccount.id + ) } +

+ ) } + + ); }; return ( 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 618549ac3c..0bee8a26e0 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 @@ -96,7 +96,6 @@ const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { isCreatingAccounts={ creatingAccounts } isCreatingAdsAccount={ isCreatingAdsAccount } isCreatingMCAccount={ isCreatingMCAccount } - googleAccount={ googleAccount } googleMCAccount={ googleMCAccount } googleAdsAccount={ googleAdsAccount } /> diff --git a/js/src/hooks/useAutoCreateAdsMCAccounts.js b/js/src/hooks/useAutoCreateAdsMCAccounts.js index d5e21de5c7..f41fb79058 100644 --- a/js/src/hooks/useAutoCreateAdsMCAccounts.js +++ b/js/src/hooks/useAutoCreateAdsMCAccounts.js @@ -43,7 +43,7 @@ const useAutoCreateAdsMCAccounts = () => { const { existingAccounts: existingAdsAccount, - isResolving: isResolvingExistingAdsAccount, + hasFinishedResolution: hasFinishedResolutionForExistingAdsAccount, } = useExistingGoogleAdsAccounts(); const [ handleCreateAccount, { response } ] = useCreateMCAccount(); @@ -82,7 +82,7 @@ const useAutoCreateAdsMCAccounts = () => { useEffect( () => { const existingAccountsResolved = - ! isResolvingExistingAdsAccount && + hasFinishedResolutionForExistingAdsAccount && hasFinishedResolutionForExistingMCAccounts; accountCreationChecksResolvedRef.current = existingAccountsResolved; @@ -100,7 +100,7 @@ const useAutoCreateAdsMCAccounts = () => { } // eslint-disable-next-line react-hooks/exhaustive-deps }, [ - isResolvingExistingAdsAccount, + hasFinishedResolutionForExistingAdsAccount, hasFinishedResolutionForExistingMCAccounts, ] ); diff --git a/js/src/hooks/useExistingGoogleAdsAccounts.js b/js/src/hooks/useExistingGoogleAdsAccounts.js index 8eda3b9a2f..1e84516710 100644 --- a/js/src/hooks/useExistingGoogleAdsAccounts.js +++ b/js/src/hooks/useExistingGoogleAdsAccounts.js @@ -15,8 +15,11 @@ const useExistingGoogleAdsAccounts = () => { const isResolving = select( STORE_KEY ).isResolving( 'getExistingGoogleAdsAccounts' ); + const hasFinishedResolution = select( STORE_KEY ).hasFinishedResolution( + 'getExistingGoogleAdsAccounts' + ); - return { existingAccounts, isResolving }; + return { existingAccounts, hasFinishedResolution, isResolving }; }, [] ); }; From 1a24f0dbb9877653e9776ed634b0cb3bb7733c7d Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 3 Oct 2024 10:15:12 +0530 Subject: [PATCH 19/27] Remove unwanted props. --- .../account-creation-description.js | 92 +++++++++---------- .../connected-google-combo-account-card.js | 10 +- .../google-combo-account-card/index.js | 2 +- 3 files changed, 52 insertions(+), 52 deletions(-) diff --git a/js/src/components/google-combo-account-card/account-creation-description.js b/js/src/components/google-combo-account-card/account-creation-description.js index 78c75005d9..86756c8a77 100644 --- a/js/src/components/google-combo-account-card/account-creation-description.js +++ b/js/src/components/google-combo-account-card/account-creation-description.js @@ -12,75 +12,75 @@ import useGoogleAccount from '.~/hooks/useGoogleAccount'; * Renders the description for the account creation card. * * @param {Object} props Props. - * @param {boolean} props.isCreatingAccounts Whether accounts are being created. * @param {boolean} props.isCreatingMCAccount Whether Merchant Center account is being created. * @param {boolean} props.isCreatingAdsAccount Whether Google Ads account is being created. + * @param {boolean} props.isLoading Whether the data is loading. * @param {Object} props.googleMCAccount Google Merchant Center account. * @param {Object} props.googleAdsAccount Google Ads account. */ const AccountCreationDescription = ( { - isCreatingAccounts, isCreatingMCAccount, isCreatingAdsAccount, + isLoading = false, googleMCAccount = {}, googleAdsAccount = {}, } ) => { const { google } = useGoogleAccount(); const getDescription = () => { - let description; - - if ( isCreatingAccounts ) { - if ( isCreatingMCAccount && isCreatingAdsAccount ) { - return ( + if ( isCreatingMCAccount && isCreatingAdsAccount ) { + return ( +

+ { __( + 'You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you.', + 'google-listings-and-ads' + ) } +

+ ); + } else if ( isCreatingAdsAccount ) { + return ( + <>

{ __( - 'You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you.', + 'You don’t have Google Ads account, so we’re creating one for you.', 'google-listings-and-ads' ) }

- ); - } else if ( isCreatingAdsAccount ) { - return ( - <> -

- { __( - 'You don’t have Google Ads account, so we’re creating one for you.', - 'google-listings-and-ads' - ) } -

- - { __( - 'Required to set up conversion measurement for your store.', - 'google-listings-and-ads' - ) } - - - ); - } else if ( isCreatingMCAccount ) { - return ( - <> -

- { __( - 'You don’t have Merchant Center account, so we’re creating one for you.', - 'google-listings-and-ads' - ) } -

- - { __( - 'Required to sync products so they show on Google.', - 'google-listings-and-ads' - ) } - - - ); - } + + { __( + 'Required to set up conversion measurement for your store.', + 'google-listings-and-ads' + ) } + + + ); + } else if ( isCreatingMCAccount ) { + return ( + <> +

+ { __( + 'You don’t have Merchant Center account, so we’re creating one for you.', + 'google-listings-and-ads' + ) } +

+ + { __( + 'Required to sync products so they show on Google.', + 'google-listings-and-ads' + ) } + + + ); + } + + if ( isLoading ) { + return null; } return ( <>

{ google?.email }

- { googleMCAccount?.id && ( + { googleMCAccount?.id && googleMCAccount.id !== 0 && (

{ sprintf( // Translators: %s is the Merchant Center ID @@ -92,7 +92,7 @@ const AccountCreationDescription = ( { ) }

) } - { googleAdsAccount?.id && ( + { googleAdsAccount?.id && googleAdsAccount.id !== 0 && (

{ sprintf( // Translators: %s is the Google Ads ID 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 0bee8a26e0..eab37bc936 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 @@ -24,12 +24,9 @@ import AccountCreationDescription from './account-creation-description'; * Renders a Google account card UI with connected account information. * It will also kickoff Ads and Merchant Center account creation if the user does not have accounts. * - * @param {Object} props React props. - * @param {{ googleAccount: object }} props.googleAccount The Google account. - * * @fires gla_google_account_connect_different_account_button_click */ -const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { +const ConnectedGoogleComboAccountCard = () => { const { googleMCAccount, hasFinishedResolution: hasFinishedResolutionForCurrentMCAccount, @@ -93,7 +90,10 @@ const ConnectedGoogleComboAccountCard = ( { googleAccount } ) => { className="gla-google-combo-account-card--connected" description={ ; + return ; } if ( isConnected && ! scope.glaRequired ) { From 2325d7a275bcb057a23acd31222aa241e0ed9bdf Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 3 Oct 2024 11:20:19 +0530 Subject: [PATCH 20/27] Use refs to remove no-deps eslint rule. --- .../account-creation-description.js | 51 +++--- .../connected-google-combo-account-card.js | 42 +++-- js/src/hooks/useAutoCreateAdsMCAccounts.js | 147 +++++++++++++----- 3 files changed, 154 insertions(+), 86 deletions(-) diff --git a/js/src/components/google-combo-account-card/account-creation-description.js b/js/src/components/google-combo-account-card/account-creation-description.js index 86756c8a77..ba85d0fdc6 100644 --- a/js/src/components/google-combo-account-card/account-creation-description.js +++ b/js/src/components/google-combo-account-card/account-creation-description.js @@ -73,6 +73,14 @@ const AccountCreationDescription = ( { ); } + const DisplayAccountInfo = ( { account, text } ) => { + if ( account?.id === undefined || account.id === 0 ) { + return null; + } + + return

{ text }

; + }; + if ( isLoading ) { return null; } @@ -80,30 +88,25 @@ const AccountCreationDescription = ( { return ( <>

{ google?.email }

- { googleMCAccount?.id && googleMCAccount.id !== 0 && ( -

- { sprintf( - // Translators: %s is the Merchant Center ID - __( - 'Merchant Center ID: %s', - 'google-listings-and-ads' - ), - googleMCAccount.id - ) } -

- ) } - { googleAdsAccount?.id && googleAdsAccount.id !== 0 && ( -

- { sprintf( - // Translators: %s is the Google Ads ID - __( - 'Google Ads ID: %s', - 'google-listings-and-ads' - ), - googleAdsAccount.id - ) } -

- ) } + + ); }; 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 eab37bc936..22c1937239 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 @@ -38,14 +38,14 @@ const ConnectedGoogleComboAccountCard = () => { } = useGoogleAdsAccount(); const { - accountsCreated, - accountCreationChecksResolved, + isCreatingAccounts, + isCreatingBothAccounts, isCreatingAdsAccount, isCreatingMCAccount, + accountCreationChecksResolved, + accountsCreated, } = useAutoCreateAdsMCAccounts(); - const isCreatingAccounts = isCreatingAdsAccount || isCreatingMCAccount; - if ( ! accountsCreated && ( ! hasFinishedResolutionForCurrentAdsAccount || @@ -55,25 +55,23 @@ const ConnectedGoogleComboAccountCard = () => { return } />; } - const creatingAccounts = - isCreatingAccounts || - ( accountsCreated && - ( ! hasFinishedResolutionForCurrentMCAccount || - ! hasFinishedResolutionForCurrentAdsAccount ) ); + const getHelper = () => { + if ( isCreatingBothAccounts ) { + return ( +

+ { __( + 'Merchant Center is required to sync products so they show on Google. Google Ads is required to set up conversion measurement for your store.', + 'google-listings-and-ads' + ) } +

+ ); + } - const getHelper = () => - isCreatingAdsAccount && - isCreatingMCAccount && ( -

- { __( - 'Merchant Center is required to sync products so they show on Google. Google Ads is required to set up conversion measurement for your store.', - 'google-listings-and-ads' - ) } -

- ); + return null; + }; const getIndicator = () => { - if ( creatingAccounts ) { + if ( isCreatingAccounts ) { return ( { description={ { * isCreatingAccountsRef - Indicates if the accounts are being created. * accountsCreatedRef - Indicates if the accounts have been created. */ - const accountCreationChecksResolvedRef = useRef( false ); - const isCreatingAccountsRef = useRef( false ); + const isCreatingBothAccountsRef = useRef( false ); const isCreatingAdsAccountsRef = useRef( false ); const isCreatingMCAccountsRef = useRef( false ); + const initHasExistingMCAAccountsRef = useRef( null ); + const initHasExistingAdsAccountsRef = useRef( null ); const accountsCreatedRef = useRef( false ); const { @@ -43,72 +44,138 @@ const useAutoCreateAdsMCAccounts = () => { const { existingAccounts: existingAdsAccount, - hasFinishedResolution: hasFinishedResolutionForExistingAdsAccount, + isResolving: isResolvingExistingAdsAccount, } = useExistingGoogleAdsAccounts(); const [ handleCreateAccount, { response } ] = useCreateMCAccount(); const [ upsertAdsAccount, { loading } ] = useUpsertAdsAccount(); - const createAccounts = async () => { - const hasExistingMCAccount = existingMCAccounts.length > 0; - const hasExistingAdsAccount = existingAdsAccount.length > 0; + const hasExistingMCAccount = existingMCAccounts?.length > 0; + const hasExistingAdsAccount = existingAdsAccount?.length > 0; + + if ( + initHasExistingMCAAccountsRef.current === null && + hasFinishedResolutionForExistingMCAccounts + ) { + initHasExistingMCAAccountsRef.current = hasExistingMCAccount; + } + + if ( + initHasExistingAdsAccountsRef.current === null && + ! isResolvingExistingAdsAccount + ) { + initHasExistingAdsAccountsRef.current = hasExistingAdsAccount; + } + + const createMCAccount = useCallback( async () => { + await handleCreateAccount(); + }, [ handleCreateAccount ] ); + + const createAdsAccount = useCallback( async () => { + await upsertAdsAccount(); + }, [ upsertAdsAccount ] ); + + const createBothAccounts = useCallback( async () => { + await createMCAccount(); + await createAdsAccount(); + }, [ createMCAccount, createAdsAccount ] ); + + const accountCreationChecksResolved = + initHasExistingAdsAccountsRef.current !== null && + initHasExistingMCAAccountsRef.current !== null; + + const shouldCreateAdsAccount = + initHasExistingAdsAccountsRef.current === false && + initHasExistingMCAAccountsRef.current === true; + + const shouldCreateMCAccount = + initHasExistingAdsAccountsRef.current === true && + initHasExistingMCAAccountsRef.current === false; + + const shouldCreateBothAccounts = + ! initHasExistingAdsAccountsRef.current && + ! initHasExistingMCAAccountsRef.current; + + const isCreatingAccounts = + isCreatingAdsAccountsRef.current || + isCreatingMCAccountsRef.current || + isCreatingBothAccountsRef.current; - isCreatingMCAccountsRef.current = ! hasExistingMCAccount; - isCreatingAdsAccountsRef.current = ! hasExistingAdsAccount; - isCreatingAccountsRef.current = - ! hasExistingAdsAccount || ! hasExistingMCAccount; - - if ( ! hasExistingMCAccount ) { - await handleCreateAccount(); + useEffect( () => { + // Ads account check + if ( isCreatingAdsAccountsRef.current === true && ! loading ) { + isCreatingAdsAccountsRef.current = false; + accountsCreatedRef.current = true; } - if ( ! hasExistingAdsAccount ) { - await upsertAdsAccount(); + // MC account check + if ( + isCreatingMCAccountsRef.current === true && + response?.status === 200 + ) { + isCreatingMCAccountsRef.current = false; + accountsCreatedRef.current = true; } - }; - useEffect( () => { + // both accounts check if ( - isCreatingAccountsRef.current === true && + isCreatingBothAccountsRef.current === true && response?.status === 200 && ! loading ) { - isCreatingMCAccountsRef.current = false; - isCreatingAdsAccountsRef.current = false; - isCreatingAccountsRef.current = false; + isCreatingBothAccountsRef.current = false; accountsCreatedRef.current = true; } }, [ response, loading ] ); useEffect( () => { - const existingAccountsResolved = - hasFinishedResolutionForExistingAdsAccount && - hasFinishedResolutionForExistingMCAccounts; + const handleCreation = async () => { + // Bail out if we haven't resolved the existing accounts checks yet or there's a creation in progress or the accounts have been created. + if ( + ! accountCreationChecksResolved || + isCreatingAccounts || + accountsCreatedRef.current + ) { + return; + } - accountCreationChecksResolvedRef.current = existingAccountsResolved; + if ( shouldCreateAdsAccount ) { + isCreatingAdsAccountsRef.current = true; + await createAdsAccount(); + return; + } - if ( - existingAccountsResolved && - isCreatingAccountsRef.current === false - ) { - const hasExistingMCAccount = existingMCAccounts?.length > 0; - const hasExistingAdsAccount = existingAdsAccount?.length > 0; + if ( shouldCreateMCAccount ) { + isCreatingMCAccountsRef.current = true; + await createMCAccount(); + return; + } - if ( ! hasExistingMCAccount || ! hasExistingAdsAccount ) { - createAccounts(); + if ( shouldCreateBothAccounts ) { + isCreatingBothAccountsRef.current = true; + await createBothAccounts(); } - } - // eslint-disable-next-line react-hooks/exhaustive-deps + }; + + handleCreation(); }, [ - hasFinishedResolutionForExistingAdsAccount, - hasFinishedResolutionForExistingMCAccounts, + accountCreationChecksResolved, + createBothAccounts, + createAdsAccount, + createMCAccount, + isCreatingAccounts, + shouldCreateAdsAccount, + shouldCreateMCAccount, + shouldCreateBothAccounts, ] ); return { - accountsCreated: accountsCreatedRef.current, - accountCreationChecksResolved: accountCreationChecksResolvedRef.current, + accountCreationChecksResolved, isCreatingAdsAccount: isCreatingAdsAccountsRef.current, isCreatingMCAccount: isCreatingMCAccountsRef.current, + isCreatingBothAccounts: isCreatingBothAccountsRef.current, + isCreatingAccounts, + accountsCreated: accountsCreatedRef.current, }; }; From a0dcb9a8b23f5a04660262e4b0f36a6ed4f0a4ea Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 3 Oct 2024 13:59:43 +0530 Subject: [PATCH 21/27] Fix: e2e tests. --- .../account-creation-description.js | 3 +- .../connected-google-combo-account-card.js | 1 + js/src/hooks/useAutoCreateAdsMCAccounts.js | 18 +-- .../setup-stepper/setup-accounts/index.js | 3 + .../specs/setup-mc/step-1-accounts.test.js | 147 ++++++++++++++++-- tests/e2e/utils/mock-requests.js | 86 ++++------ 6 files changed, 182 insertions(+), 76 deletions(-) diff --git a/js/src/components/google-combo-account-card/account-creation-description.js b/js/src/components/google-combo-account-card/account-creation-description.js index ba85d0fdc6..91c845fe39 100644 --- a/js/src/components/google-combo-account-card/account-creation-description.js +++ b/js/src/components/google-combo-account-card/account-creation-description.js @@ -19,6 +19,7 @@ import useGoogleAccount from '.~/hooks/useGoogleAccount'; * @param {Object} props.googleAdsAccount Google Ads account. */ const AccountCreationDescription = ( { + isCreatingBothAccounts, isCreatingMCAccount, isCreatingAdsAccount, isLoading = false, @@ -28,7 +29,7 @@ const AccountCreationDescription = ( { const { google } = useGoogleAccount(); const getDescription = () => { - if ( isCreatingMCAccount && isCreatingAdsAccount ) { + if ( isCreatingBothAccounts ) { return (

{ __( 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 22c1937239..d58944cd63 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 @@ -92,6 +92,7 @@ const ConnectedGoogleComboAccountCard = () => { ! hasFinishedResolutionForCurrentAdsAccount || ! hasFinishedResolutionForCurrentMCAccount } + isCreatingBothAccounts={ isCreatingBothAccounts } isCreatingAdsAccount={ isCreatingAdsAccount } isCreatingMCAccount={ isCreatingMCAccount } googleMCAccount={ googleMCAccount } diff --git a/js/src/hooks/useAutoCreateAdsMCAccounts.js b/js/src/hooks/useAutoCreateAdsMCAccounts.js index 06935d2589..4a1ea0932a 100644 --- a/js/src/hooks/useAutoCreateAdsMCAccounts.js +++ b/js/src/hooks/useAutoCreateAdsMCAccounts.js @@ -33,7 +33,7 @@ const useAutoCreateAdsMCAccounts = () => { const isCreatingBothAccountsRef = useRef( false ); const isCreatingAdsAccountsRef = useRef( false ); const isCreatingMCAccountsRef = useRef( false ); - const initHasExistingMCAAccountsRef = useRef( null ); + const initHasExistingMCAccountsRef = useRef( null ); const initHasExistingAdsAccountsRef = useRef( null ); const accountsCreatedRef = useRef( false ); @@ -44,7 +44,7 @@ const useAutoCreateAdsMCAccounts = () => { const { existingAccounts: existingAdsAccount, - isResolving: isResolvingExistingAdsAccount, + hasFinishedResolution: hasFinishedResolutionForExistingAdsAccount, } = useExistingGoogleAdsAccounts(); const [ handleCreateAccount, { response } ] = useCreateMCAccount(); @@ -54,15 +54,15 @@ const useAutoCreateAdsMCAccounts = () => { const hasExistingAdsAccount = existingAdsAccount?.length > 0; if ( - initHasExistingMCAAccountsRef.current === null && + initHasExistingMCAccountsRef.current === null && hasFinishedResolutionForExistingMCAccounts ) { - initHasExistingMCAAccountsRef.current = hasExistingMCAccount; + initHasExistingMCAccountsRef.current = hasExistingMCAccount; } if ( initHasExistingAdsAccountsRef.current === null && - ! isResolvingExistingAdsAccount + hasFinishedResolutionForExistingAdsAccount ) { initHasExistingAdsAccountsRef.current = hasExistingAdsAccount; } @@ -82,19 +82,19 @@ const useAutoCreateAdsMCAccounts = () => { const accountCreationChecksResolved = initHasExistingAdsAccountsRef.current !== null && - initHasExistingMCAAccountsRef.current !== null; + initHasExistingMCAccountsRef.current !== null; const shouldCreateAdsAccount = initHasExistingAdsAccountsRef.current === false && - initHasExistingMCAAccountsRef.current === true; + initHasExistingMCAccountsRef.current === true; const shouldCreateMCAccount = initHasExistingAdsAccountsRef.current === true && - initHasExistingMCAAccountsRef.current === false; + initHasExistingMCAccountsRef.current === false; const shouldCreateBothAccounts = ! initHasExistingAdsAccountsRef.current && - ! initHasExistingMCAAccountsRef.current; + ! initHasExistingMCAccountsRef.current; const isCreatingAccounts = isCreatingAdsAccountsRef.current || 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 d4bfc68f86..3288e46f61 100644 --- a/js/src/setup-mc/setup-stepper/setup-accounts/index.js +++ b/js/src/setup-mc/setup-stepper/setup-accounts/index.js @@ -127,6 +127,9 @@ const SetupAccounts = ( props ) => { ( googleMCAccount?.status === 'incomplete' && googleMCAccount?.step === 'link_ads' ) ); + console.log('isGoogleAdsReady', isGoogleAdsReady); + console.log('isGoogleMCReady', isGoogleMCReady); + const isContinueButtonDisabled = ! ( hasFinishedResolution && isGoogleAdsReady && diff --git a/tests/e2e/specs/setup-mc/step-1-accounts.test.js b/tests/e2e/specs/setup-mc/step-1-accounts.test.js index de4c235120..432b159729 100644 --- a/tests/e2e/specs/setup-mc/step-1-accounts.test.js +++ b/tests/e2e/specs/setup-mc/step-1-accounts.test.js @@ -223,23 +223,70 @@ test.describe( 'Set up accounts', () => { await setUpAccountsPage.mockJetpackConnected(); await setUpAccountsPage.mockGoogleConnected(); - await setupAdsAccountPage.fulfillAdsAccountsRequests( [ + await page.route( /\/wc\/gla\/mc\/accounts\b/, ( route ) => { + if ( route.request().method() === 'POST' ) { + // Do nothing. + } else { + route.continue(); + } + } ); + + await page.route( /\/wc\/gla\/ads\/accounts\b/, ( route ) => { + if ( route.request().method() === 'POST' ) { + // Do nothing. + } else { + route.continue(); + } + } ); + + await setupAdsAccountPage.fulfillAdsAccounts( + [ + [], + { + id: 78787878, + name: 'gla', + }, + ], + 200, + [ 'GET' ] + ); + + await setupAdsAccountPage.fulfillMCAccounts( + [ + [], + { + id: 5432178, + name: null, + subaccount: null, + domain: null, + }, + ], + 200, + 'GET' + ); + + await setUpAccountsPage.fulfillAdsConnection( [ { id: 0, currency: 'USD', - status: 'disconnected', + status: 'approved', symbol: '$', }, { - id: 119119119, + id: 78787878, currency: 'USD', - status: 'disconnected', + status: 'approved', symbol: '$', }, ] ); - await setupAdsAccountPage.fulfillMCAccountsRequests( [ - [], + await setUpAccountsPage.fulfillMCConnection( [ + { + id: 0, + name: null, + subaccount: null, + domain: null, + }, { id: 5432178, name: null, @@ -262,20 +309,98 @@ test.describe( 'Set up accounts', () => { } ); test( 'should see the merchant center id and ads account id if connected', async () => { + await setUpAccountsPage.mockJetpackConnected(); await setUpAccountsPage.mockGoogleConnected(); - await setUpAccountsPage.mockAdsAccountConnected(); - await setUpAccountsPage.mockMCConnected(); await setUpAccountsPage.goto(); + await setupAdsAccountPage.fulfillAdsAccounts( + { + message: + 'Account must be accepted before completing setup.', + has_access: false, + step: 'account_access', + invite_link: 'https://ads.google.com/nav/', + }, + 428, + [ 'POST' ] + ); + + await setupAdsAccountPage.fulfillMCAccounts( + { + id: 5432178, + name: null, + subaccount: null, + domain: null, + }, + 200, + [ 'POST' ] + ); + + await setupAdsAccountPage.fulfillAdsAccounts( + [ + [], + { + id: 78787878, + name: 'gla', + }, + ], + 200, + [ 'GET' ] + ); + + await setupAdsAccountPage.fulfillMCAccounts( + [ + [], + { + id: 5432178, + name: null, + subaccount: null, + domain: null, + }, + ], + 200, + 'GET' + ); + + await setUpAccountsPage.fulfillAdsConnection( [ + { + id: 0, + currency: 'USD', + status: 'approved', + symbol: '$', + }, + { + id: 78787878, + currency: 'USD', + status: 'approved', + symbol: '$', + }, + ] ); + + await setUpAccountsPage.fulfillMCConnection( [ + { + id: 0, + name: null, + subaccount: null, + domain: null, + }, + { + id: 5432178, + name: null, + subaccount: null, + domain: null, + }, + ] ); + const googleAccountCard = setUpAccountsPage.getGoogleAccountCard(); await expect( - googleAccountCard.getByText( 'Merchant Center ID: 1234', { + googleAccountCard.getByText( 'Merchant Center ID: 5432178', { exact: true, } ) ).toBeVisible(); await expect( - googleAccountCard.getByText( 'Google Ads ID: 12345', { + googleAccountCard.getByText( 'Google Ads ID: 78787878', { exact: true, } ) ).toBeVisible(); @@ -326,6 +451,8 @@ test.describe( 'Set up accounts', () => { test.describe( 'When all accounts are connected', async () => { test.beforeAll( async () => { + await setupAdsAccountPage.mockJetpackConnected(); + await setUpAccountsPage.mockGoogleConnected(); await setupAdsAccountPage.mockAdsAccountConnected(); await setupAdsAccountPage.mockAdsStatusClaimed(); await setUpAccountsPage.mockMCConnected(); diff --git a/tests/e2e/utils/mock-requests.js b/tests/e2e/utils/mock-requests.js index a94caebe59..477c608057 100644 --- a/tests/e2e/utils/mock-requests.js +++ b/tests/e2e/utils/mock-requests.js @@ -21,49 +21,38 @@ export default class MockRequests { * @return {Promise} */ async fulfillRequest( url, payload, status = 200, methods = [] ) { + let callCount = 0; + await this.page.route( url, ( route ) => { if ( - methods.length === 0 || - methods.includes( route.request().method() ) + ( Array.isArray( payload ) && callCount < payload.length ) || // Ensure there are payloads to fulfill + ( ! Array.isArray( payload ) && callCount === 0 ) // For single payload scenario ) { - route.fulfill( { - status, - content: 'application/json', - headers: { 'Access-Control-Allow-Origin': '*' }, - body: JSON.stringify( payload ), - } ); + if ( + methods.length === 0 || + methods.includes( route.request().method() ) + ) { + // Handle single or multiple payloads + const currentPayload = Array.isArray( payload ) + ? payload[ callCount ] + : payload; + + route.fulfill( { + status, + contentType: 'application/json', + headers: { 'Access-Control-Allow-Origin': '*' }, + body: JSON.stringify( currentPayload ), + } ); + callCount += 1; + } else { + route.fallback(); + } } else { route.fallback(); } } ); } - /** - * Send the different response for each time the route is called. - * - * @param {RegExp|string} url The url to fulfill. - * @param {Array} payloads The payload to send. - * @param {number} status The HTTP status in the response. - * @return {Promise} - */ - async fulfillRequests( url, payloads = [], status = 200 ) { - let callCount = 0; - await this.page.route( url, ( route ) => { - if ( callCount < payloads.length ) { - route.fulfill( { - status, - contentType: 'application/json', - headers: { 'Access-Control-Allow-Origin': '*' }, - body: JSON.stringify( payloads[ callCount ] ), - } ); - } else { - // Optionally handle additional calls beyond the responses array - route.continue(); - } - callCount += 1; - } ); - } - /** * Fulfill the WC options default country request. * @@ -136,16 +125,6 @@ export default class MockRequests { ); } - /** - * Fulfill the MC accounts requests. - * - * @param {Object} payloads - * @return {Promise} - */ - async fulfillMCAccountsRequests( payloads ) { - await this.fulfillRequests( /\/wc\/gla\/mc\/accounts\b/, payloads ); - } - /** * Fulfill the MC accounts claim-overwrite request. * @@ -254,18 +233,13 @@ export default class MockRequests { * @param {Object} payload * @return {Promise} */ - async fulfillAdsAccounts( payload ) { - await this.fulfillRequest( /\/wc\/gla\/ads\/accounts\b/, payload ); - } - - /** - * Fulfill the Ads Account requests. - * - * @param {Object} payloads - * @return {Promise} - */ - async fulfillAdsAccountsRequests( payloads ) { - await this.fulfillRequests( /\/wc\/gla\/ads\/accounts\b/, payloads ); + async fulfillAdsAccounts( payload, status = 200, methods = [] ) { + await this.fulfillRequest( + /\/wc\/gla\/ads\/accounts\b/, + payload, + status, + methods + ); } /** From ea3a30f83d6be08e2b1b0b42eb0a476324dd8542 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 3 Oct 2024 15:50:33 +0530 Subject: [PATCH 22/27] Fix: hook tests. --- .../hooks/useAutoCreateAdsMCAccounts.test.js | 19 +++++-------------- .../setup-stepper/setup-accounts/index.js | 3 --- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/js/src/hooks/useAutoCreateAdsMCAccounts.test.js b/js/src/hooks/useAutoCreateAdsMCAccounts.test.js index 844b968ae8..152bd20f1e 100644 --- a/js/src/hooks/useAutoCreateAdsMCAccounts.test.js +++ b/js/src/hooks/useAutoCreateAdsMCAccounts.test.js @@ -41,7 +41,7 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { } ); useExistingGoogleAdsAccounts.mockReturnValue( { existingAccounts: [], - isResolving: false, + hasFinishedResolution: true, } ); } ); @@ -53,7 +53,7 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { } ); useExistingGoogleAdsAccounts.mockReturnValue( { existingAccounts: [ { id: 1 } ], - isResolving: false, + hasFinishedResolution: false, } ); const { result } = renderHook( () => useCreateAccounts() ); @@ -85,8 +85,8 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { rerender(); // Trigger the effect that begins account creation } ); - expect( result.current.isCreatingAdsAccount ).toBe( true ); - expect( result.current.isCreatingMCAccount ).toBe( true ); + expect( result.current.isCreatingAdsAccount ).toBe( false ); + expect( result.current.isCreatingMCAccount ).toBe( false ); await act( async () => { rerender(); @@ -102,14 +102,9 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { } ); it( 'should create Merchant Center account only when there is no existing MC account, but there is an existing Ads account', async () => { - // Simulate no existing MC account but an existing Ads account - useExistingGoogleMCAccounts.mockReturnValue( { - data: [], // No existing MC account - hasFinishedResolution: true, - } ); useExistingGoogleAdsAccounts.mockReturnValue( { existingAccounts: [ { id: 1 } ], // Existing Ads account - isResolving: false, + hasFinishedResolution: true, } ); // Step 1: Initial render - No response, loading is true @@ -162,10 +157,6 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { data: [ { id: 1 } ], // Existing MC account hasFinishedResolution: true, } ); - useExistingGoogleAdsAccounts.mockReturnValue( { - existingAccounts: [], // No existing Ads account - isResolving: false, - } ); // Step 1: Initial render - No response, loading is true for Ads account creation useUpsertAdsAccount.mockReturnValueOnce( [ 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 3288e46f61..d4bfc68f86 100644 --- a/js/src/setup-mc/setup-stepper/setup-accounts/index.js +++ b/js/src/setup-mc/setup-stepper/setup-accounts/index.js @@ -127,9 +127,6 @@ const SetupAccounts = ( props ) => { ( googleMCAccount?.status === 'incomplete' && googleMCAccount?.step === 'link_ads' ) ); - console.log('isGoogleAdsReady', isGoogleAdsReady); - console.log('isGoogleMCReady', isGoogleMCReady); - const isContinueButtonDisabled = ! ( hasFinishedResolution && isGoogleAdsReady && From 58738f81840efddac41580bb82089c5203c34e08 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 3 Oct 2024 15:54:54 +0530 Subject: [PATCH 23/27] Lint fix. --- .../google-combo-account-card/account-creation-description.js | 1 + 1 file changed, 1 insertion(+) diff --git a/js/src/components/google-combo-account-card/account-creation-description.js b/js/src/components/google-combo-account-card/account-creation-description.js index 91c845fe39..a6b53d6354 100644 --- a/js/src/components/google-combo-account-card/account-creation-description.js +++ b/js/src/components/google-combo-account-card/account-creation-description.js @@ -12,6 +12,7 @@ import useGoogleAccount from '.~/hooks/useGoogleAccount'; * Renders the description for the account creation card. * * @param {Object} props Props. + * @param {boolean} props.isCreatingBothAccounts Whether both, MC and Ads accounts are being created. * @param {boolean} props.isCreatingMCAccount Whether Merchant Center account is being created. * @param {boolean} props.isCreatingAdsAccount Whether Google Ads account is being created. * @param {boolean} props.isLoading Whether the data is loading. From a2b5f196e13da1ed98fad6e2f15753bec53225ed Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 3 Oct 2024 16:05:35 +0530 Subject: [PATCH 24/27] Remove unnecessary act in tests. --- .../hooks/useAutoCreateAdsMCAccounts.test.js | 29 ++++++------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/js/src/hooks/useAutoCreateAdsMCAccounts.test.js b/js/src/hooks/useAutoCreateAdsMCAccounts.test.js index 152bd20f1e..2fdc806d34 100644 --- a/js/src/hooks/useAutoCreateAdsMCAccounts.test.js +++ b/js/src/hooks/useAutoCreateAdsMCAccounts.test.js @@ -1,4 +1,3 @@ -/* eslint-disable testing-library/no-unnecessary-act */ /** * External dependencies */ @@ -81,13 +80,12 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { { loading: false }, ] ); - await act( async () => { - rerender(); // Trigger the effect that begins account creation - } ); + rerender(); expect( result.current.isCreatingAdsAccount ).toBe( false ); expect( result.current.isCreatingMCAccount ).toBe( false ); + // eslint-disable-next-line testing-library/no-unnecessary-act await act( async () => { rerender(); } ); @@ -119,10 +117,8 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { expect( result.current.isCreatingAdsAccount ).toBe( false ); expect( result.current.isCreatingMCAccount ).toBe( false ); - // Trigger the effect that starts account creation - await act( async () => { - rerender(); // Simulate the effect firing - } ); + // Trigger the effect that starts account creation. + rerender(); // Step 2: At this point, MC account creation should have started expect( result.current.isCreatingMCAccount ).toBe( true ); @@ -136,10 +132,8 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { { response: { status: 200 }, loading: false }, // Now account creation is complete ] ); - // Trigger a rerender to simulate the async function resolving - await act( async () => { - rerender(); // Re-render after response has been updated - } ); + // Trigger a rerender to simulate the async function resolving. + rerender(); // Step 4: Final assertions after account creation has completed expect( result.current.isCreatingAdsAccount ).toBe( false ); @@ -170,10 +164,7 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { expect( result.current.isCreatingAdsAccount ).toBe( false ); expect( result.current.isCreatingMCAccount ).toBe( false ); - // Trigger the effect that starts account creation - await act( async () => { - rerender(); // Simulate the effect firing - } ); + rerender(); // Simulate the effect firing. // Step 2: At this point, Ads account creation should have started expect( result.current.isCreatingAdsAccount ).toBe( true ); @@ -187,10 +178,8 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => { { response: { status: 200 }, loading: false }, // Now Ads account creation is complete ] ); - // Trigger a rerender to simulate the async function resolving - await act( async () => { - rerender(); // Re-render after response has been updated - } ); + // Trigger a rerender to simulate the async function resolving. + rerender(); // Step 4: Final assertions after Ads account creation has completed expect( result.current.accountsCreated ).toBe( true ); // The account has been created From c4f0d2b73f5a1d56c626ad821548b73c8a013e59 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 3 Oct 2024 21:13:21 +0530 Subject: [PATCH 25/27] Move hooks to relevant component. --- .../account-creation-description.js | 123 ---------------- .../account-info.js | 16 +++ .../account-creation-description/index.js | 131 ++++++++++++++++++ .../connected-google-combo-account-card.js | 24 +--- 4 files changed, 149 insertions(+), 145 deletions(-) delete mode 100644 js/src/components/google-combo-account-card/account-creation-description.js create mode 100644 js/src/components/google-combo-account-card/account-creation-description/account-info.js create mode 100644 js/src/components/google-combo-account-card/account-creation-description/index.js diff --git a/js/src/components/google-combo-account-card/account-creation-description.js b/js/src/components/google-combo-account-card/account-creation-description.js deleted file mode 100644 index a6b53d6354..0000000000 --- a/js/src/components/google-combo-account-card/account-creation-description.js +++ /dev/null @@ -1,123 +0,0 @@ -/** - * External dependencies - */ -import { __, sprintf } from '@wordpress/i18n'; - -/** - * Internal dependencies - */ -import useGoogleAccount from '.~/hooks/useGoogleAccount'; - -/** - * Renders the description for the account creation card. - * - * @param {Object} props Props. - * @param {boolean} props.isCreatingBothAccounts Whether both, MC and Ads accounts are being created. - * @param {boolean} props.isCreatingMCAccount Whether Merchant Center account is being created. - * @param {boolean} props.isCreatingAdsAccount Whether Google Ads account is being created. - * @param {boolean} props.isLoading Whether the data is loading. - * @param {Object} props.googleMCAccount Google Merchant Center account. - * @param {Object} props.googleAdsAccount Google Ads account. - */ -const AccountCreationDescription = ( { - isCreatingBothAccounts, - isCreatingMCAccount, - isCreatingAdsAccount, - isLoading = false, - googleMCAccount = {}, - googleAdsAccount = {}, -} ) => { - const { google } = useGoogleAccount(); - - const getDescription = () => { - if ( isCreatingBothAccounts ) { - return ( -

- { __( - 'You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you.', - 'google-listings-and-ads' - ) } -

- ); - } else if ( isCreatingAdsAccount ) { - return ( - <> -

- { __( - 'You don’t have Google Ads account, so we’re creating one for you.', - 'google-listings-and-ads' - ) } -

- - { __( - 'Required to set up conversion measurement for your store.', - 'google-listings-and-ads' - ) } - - - ); - } else if ( isCreatingMCAccount ) { - return ( - <> -

- { __( - 'You don’t have Merchant Center account, so we’re creating one for you.', - 'google-listings-and-ads' - ) } -

- - { __( - 'Required to sync products so they show on Google.', - 'google-listings-and-ads' - ) } - - - ); - } - - const DisplayAccountInfo = ( { account, text } ) => { - if ( account?.id === undefined || account.id === 0 ) { - return null; - } - - return

{ text }

; - }; - - if ( isLoading ) { - return null; - } - - return ( - <> -

{ google?.email }

- - - - ); - }; - - return ( -
- { getDescription() } -
- ); -}; - -export default AccountCreationDescription; diff --git a/js/src/components/google-combo-account-card/account-creation-description/account-info.js b/js/src/components/google-combo-account-card/account-creation-description/account-info.js new file mode 100644 index 0000000000..673c16a1d4 --- /dev/null +++ b/js/src/components/google-combo-account-card/account-creation-description/account-info.js @@ -0,0 +1,16 @@ +/** + * Display account info. + * + * @param {Object} props Props. + * @param {Object} props.account Account object. + * @param {string} props.text Text to display. + */ +const AccountInfo = ( { account, text } ) => { + if ( ! account?.id ) { + return null; + } + + return

{ text }

; +}; + +export default AccountInfo; diff --git a/js/src/components/google-combo-account-card/account-creation-description/index.js b/js/src/components/google-combo-account-card/account-creation-description/index.js new file mode 100644 index 0000000000..bb5a7ec23d --- /dev/null +++ b/js/src/components/google-combo-account-card/account-creation-description/index.js @@ -0,0 +1,131 @@ +/** + * External dependencies + */ +import { __, sprintf } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import useGoogleAccount from '.~/hooks/useGoogleAccount'; +import useGoogleMCAccount from '.~/hooks/useGoogleMCAccount'; +import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; +import AccountInfo from './account-info'; + +/** + * Renders the description for the account creation card. + * + * @param {Object} props Props. + * @param {Object} props.accountsCreated Whether accounts have been created. + * @param {boolean} props.isCreatingBothAccounts Whether both, MC and Ads accounts are being created. + * @param {boolean} props.isCreatingMCAccount Whether Merchant Center account is being created. + * @param {boolean} props.isCreatingAdsAccount Whether Google Ads account is being created. + */ +const AccountCreationDescription = ( { + accountsCreated, + isCreatingBothAccounts, + isCreatingMCAccount, + isCreatingAdsAccount, +} ) => { + const { google } = useGoogleAccount(); + const { + googleMCAccount, + hasFinishedResolution: hasFinishedResolutionForCurrentMCAccount, + } = useGoogleMCAccount(); + + const { + googleAdsAccount, + hasFinishedResolution: hasFinishedResolutionForCurrentAdsAccount, + } = useGoogleAdsAccount(); + + const isLoadingData = + accountsCreated && + ( ! hasFinishedResolutionForCurrentMCAccount || + ! hasFinishedResolutionForCurrentAdsAccount ); + + const getDescription = () => { + if ( + isLoadingData || + isCreatingBothAccounts || + isCreatingMCAccount || + isCreatingAdsAccount + ) { + if ( isCreatingBothAccounts ) { + return ( +

+ { __( + 'You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you.', + 'google-listings-and-ads' + ) } +

+ ); + } else if ( isCreatingAdsAccount ) { + return ( + <> +

+ { __( + 'You don’t have Google Ads account, so we’re creating one for you.', + 'google-listings-and-ads' + ) } +

+ + { __( + 'Required to set up conversion measurement for your store.', + 'google-listings-and-ads' + ) } + + + ); + } else if ( isCreatingMCAccount ) { + return ( + <> +

+ { __( + 'You don’t have Merchant Center account, so we’re creating one for you.', + 'google-listings-and-ads' + ) } +

+ + { __( + 'Required to sync products so they show on Google.', + 'google-listings-and-ads' + ) } + + + ); + } + } + + return ( + <> +

{ google?.email }

+ + + + ); + }; + + return ( +
+ { getDescription() } +
+ ); +}; + +export default AccountCreationDescription; 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 d58944cd63..00135cae14 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 @@ -27,16 +27,6 @@ import AccountCreationDescription from './account-creation-description'; * @fires gla_google_account_connect_different_account_button_click */ const ConnectedGoogleComboAccountCard = () => { - const { - googleMCAccount, - hasFinishedResolution: hasFinishedResolutionForCurrentMCAccount, - } = useGoogleMCAccount(); - - const { - googleAdsAccount, - hasFinishedResolution: hasFinishedResolutionForCurrentAdsAccount, - } = useGoogleAdsAccount(); - const { isCreatingAccounts, isCreatingBothAccounts, @@ -46,12 +36,7 @@ const ConnectedGoogleComboAccountCard = () => { accountsCreated, } = useAutoCreateAdsMCAccounts(); - if ( - ! accountsCreated && - ( ! hasFinishedResolutionForCurrentAdsAccount || - ! hasFinishedResolutionForCurrentMCAccount || - ! accountCreationChecksResolved ) - ) { + if ( ! accountCreationChecksResolved ) { return } />; } @@ -88,15 +73,10 @@ const ConnectedGoogleComboAccountCard = () => { className="gla-google-combo-account-card--connected" description={ } helper={ getHelper() } From 0d5130f8bc1083fc79087ca2ea65d324542d49fb Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 3 Oct 2024 21:24:56 +0530 Subject: [PATCH 26/27] Fix js linting. --- .../connected-google-combo-account-card.js | 2 -- 1 file changed, 2 deletions(-) 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 00135cae14..bc554cfa8f 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 @@ -7,8 +7,6 @@ import { __ } from '@wordpress/i18n'; * Internal dependencies */ import AccountCard, { APPEARANCE } from '../account-card'; -import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; -import useGoogleMCAccount from '.~/hooks/useGoogleMCAccount'; import AppSpinner from '../app-spinner'; import useAutoCreateAdsMCAccounts from '../../hooks/useAutoCreateAdsMCAccounts'; import LoadingLabel from '../loading-label/loading-label'; From 896c681e2d730282536ca014b54bb7264eebd26f Mon Sep 17 00:00:00 2001 From: asvinb Date: Thu, 3 Oct 2024 20:30:10 +0400 Subject: [PATCH 27/27] Load correct scss file in appropriate component. --- .../connect-google-combo-account-card.js | 1 - .../connected-google-combo-account-card.js | 1 + ...ccount-card.scss => connected-google-combo-account-card.scss} | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename js/src/components/google-combo-account-card/{connect-google-combo-account-card.scss => connected-google-combo-account-card.scss} (100%) diff --git a/js/src/components/google-combo-account-card/connect-google-combo-account-card.js b/js/src/components/google-combo-account-card/connect-google-combo-account-card.js index dfff4dd2c9..2537700494 100644 --- a/js/src/components/google-combo-account-card/connect-google-combo-account-card.js +++ b/js/src/components/google-combo-account-card/connect-google-combo-account-card.js @@ -14,7 +14,6 @@ import AppButton from '.~/components/app-button'; import readMoreLink from '../google-account-card/read-more-link'; import useGoogleConnectFlow from '../google-account-card/use-google-connect-flow'; import AppDocumentationLink from '../app-documentation-link'; -import './connect-google-combo-account-card.scss'; /** * @param {Object} props React props 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 bc554cfa8f..f0a752de2b 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 @@ -11,6 +11,7 @@ import AppSpinner from '../app-spinner'; import useAutoCreateAdsMCAccounts from '../../hooks/useAutoCreateAdsMCAccounts'; import LoadingLabel from '../loading-label/loading-label'; import AccountCreationDescription from './account-creation-description'; +import './connected-google-combo-account-card.scss'; /** * Clicking on the "connect to a different Google account" button. diff --git a/js/src/components/google-combo-account-card/connect-google-combo-account-card.scss b/js/src/components/google-combo-account-card/connected-google-combo-account-card.scss similarity index 100% rename from js/src/components/google-combo-account-card/connect-google-combo-account-card.scss rename to js/src/components/google-combo-account-card/connected-google-combo-account-card.scss