CR feedback.

woocommerce · Oct 24, 2024 · 9ea122a · 9ea122a
1 parent 5dc7506
commit 9ea122a
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 137 deletions.
diff --git a/js/src/components/google-combo-account-card/claim-ads-account/claim-ads-account-button.js b/js/src/components/google-combo-account-card/claim-ads-account/claim-ads-account-button.js
diff --git a/js/src/components/google-combo-account-card/claim-ads-account/claim-ads-account.js b/js/src/components/google-combo-account-card/claim-ads-account/claim-ads-account.js
@@ -1,71 +1,53 @@
 /**
  * External dependencies
  */
-import { useEffect, useRef, useState } from '@wordpress/element';
+import { useCallback, useEffect, useState } from '@wordpress/element';
 import { __ } from '@wordpress/i18n';
 import { Flex, FlexBlock } from '@wordpress/components';
 
 /**
  * Internal dependencies
  */
-import ClaimAdsAccountButton from './claim-ads-account-button';
+import ClaimAccountButton from '.~/components/google-ads-account-card/claim-account-button';
 import Section from '.~/wcdl/section';
-import useWindowFocus from '.~/hooks/useWindowFocus';
 import useGoogleAdsAccountStatus from '.~/hooks/useGoogleAdsAccountStatus';
 import { useAppDispatch } from '.~/data';
-import './claim-ads-account.scss';
 import useUpsertAdsAccount from '.~/hooks/useUpsertAdsAccount';
+import useWindowFocusCallbackIntervalEffect from '.~/hooks/useWindowFocusCallbackIntervalEffect';
+import './claim-ads-account.scss';
+import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount';
 
 /**
  * ClaimAdsAccount component.
  *
  * @return {JSX.Element} ClaimAdsAccount component.
  */
 const ClaimAdsAccount = () => {
-	const claimButtonClickedRef = useRef( false );
-	const claimInProgressRef = useRef( false );
+	const [ updating, setUpdating ] = useState( false );
+	const { fetchGoogleAdsAccountStatus } = useAppDispatch();
+	const { googleAdsAccount } = useGoogleAdsAccount();
+	const { hasAccess, step } = useGoogleAdsAccountStatus();
+	const [ upsertAdsAccount ] = useUpsertAdsAccount();
 
-	const [ , forceUpdate ] = useState( 0 );
-	const isWindowFocused = useWindowFocus();
-	const { hasAccess, hasFinishedResolution, inviteLink, step } =
-		useGoogleAdsAccountStatus();
-	const [ upsertAdsAccount, { loading } ] = useUpsertAdsAccount();
-	const { invalidateResolution } = useAppDispatch();
+	const checkUpdatedStatus = useCallback( async () => {
+		setUpdating( true );
+		await fetchGoogleAdsAccountStatus();
+		setUpdating( false );
+	}, [ fetchGoogleAdsAccountStatus ] );
 
-	useEffect( () => {
-		if ( isWindowFocused && claimButtonClickedRef.current ) {
-			invalidateResolution( 'getGoogleAdsAccountStatus' );
-			forceUpdate( ( prev ) => prev + 1 );
-		}
-	}, [ isWindowFocused, invalidateResolution ] );
+	useWindowFocusCallbackIntervalEffect( checkUpdatedStatus, 30 );
 
 	useEffect( () => {
-		const handlePostClaimActions = async () => {
-			await upsertAdsAccount();
-			claimInProgressRef.current = false;
-		};
-
-		if (
-			hasAccess === true &&
-			step === 'conversion_action' &&
-			! claimInProgressRef.current
-		) {
-			claimInProgressRef.current = true;
-			handlePostClaimActions();
-		}
-
-		if ( claimButtonClickedRef.current && hasFinishedResolution ) {
-			claimButtonClickedRef.current = false;
+		if ( hasAccess === true && step === 'conversion_action' ) {
+			upsertAdsAccount();
 		}
-	}, [ hasAccess, step, upsertAdsAccount, hasFinishedResolution ] );
+	}, [ hasAccess, step, upsertAdsAccount ] );
 
-	const isLoading =
-		loading || ( ! hasFinishedResolution && claimButtonClickedRef.current );
+	const shouldClaimGoogleAdsAccount = Boolean(
+		googleAdsAccount.id && hasAccess === false
+	);
 
-	if (
-		hasAccess ||
-		( ! hasFinishedResolution && ! claimButtonClickedRef.current )
-	) {
+	if ( ! shouldClaimGoogleAdsAccount ) {
 		return null;
 	}
 
@@ -92,12 +74,20 @@ const ClaimAdsAccount = () => {
 								'google-listings-and-ads'
 							) }
 						</p>
-						<ClaimAdsAccountButton
-							inviteLink={ inviteLink }
-							loading={ isLoading }
-							onClick={ () => {
-								claimButtonClickedRef.current = true;
-							} }
+						<ClaimAccountButton
+							loading={ updating }
+							text={
+								updating
+									? __(
+											'Updating…',
+											'google-listings-and-ads'
+									  )
+									: __(
+											'Claim your Google Ads account',
+											'google-listings-and-ads'
+									  )
+							}
+							isPrimary={ ! updating }
 						/>
 					</div>
 				</FlexBlock>

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
@@ -3,22 +3,12 @@
  */
 import AccountCard, { APPEARANCE } from '../account-card';
 import AccountDetails from './account-details';
-<<<<<<< HEAD
-import AppSpinner from '../app-spinner';
 import ClaimAdsAccount from './claim-ads-account';
 import ConversionMeasurementNotice from './conversion-measurement-notice';
-=======
->>>>>>> feature/2567-kickoff-mc-ads-account-creation
 import Indicator from './indicator';
 import getAccountCreationTexts from './getAccountCreationTexts';
 import SpinnerCard from '.~/components/spinner-card';
 import useAutoCreateAdsMCAccounts from '.~/hooks/useAutoCreateAdsMCAccounts';
-<<<<<<< HEAD
-import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount';
-import useGoogleMCAccount from '.~/hooks/useGoogleMCAccount';
-import { GOOGLE_ADS_ACCOUNT_STATUS } from '.~/constants';
-=======
->>>>>>> feature/2567-kickoff-mc-ads-account-creation
 import './connected-google-combo-account-card.scss';
 
 /**
@@ -36,13 +26,15 @@ const ConnectedGoogleComboAccountCard = () => {
 	return (
 		<>
 			<AccountCard
-			appearance={ APPEARANCE.GOOGLE }
-			alignIcon="top"
-			className="gla-google-combo-account-card--connected"
-			description={ text || <AccountDetails /> }
-			helper={ subText }
-			indicator={ <Indicator showSpinner={ Boolean( creatingWhich ) } /> }
-		>
+				appearance={ APPEARANCE.GOOGLE }
+				alignIcon="top"
+				className="gla-google-combo-account-card--connected"
+				description={ text || <AccountDetails /> }
+				helper={ subText }
+				indicator={
+					<Indicator showSpinner={ Boolean( creatingWhich ) } />
+				}
+			>
 				<ConversionMeasurementNotice />
 				<ClaimAdsAccount />
 			</AccountCard>