Add tests for existing accounts.

woocommerce · Oct 17, 2024 · 79eddb9 · 79eddb9
1 parent 747097d
commit 79eddb9
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 36 deletions.
diff --git a/js/src/hooks/useAutoCreateAdsMCAccounts.js b/js/src/hooks/useAutoCreateAdsMCAccounts.js
@@ -37,6 +37,7 @@ const useAutoCreateAdsMCAccounts = () => {
 	const { accountsCreated, isCreatingWhichAccount } = accountsState;
 	const initHasExistingMCAccountsRef = useRef( null );
 	const initHasExistingAdsAccountsRef = useRef( null );
+	const shouldCreateAccounts = useRef();
 
 	const {
 		data: existingMCAccounts,
@@ -79,24 +80,35 @@ const useAutoCreateAdsMCAccounts = () => {
 		initHasExistingAdsAccountsRef.current = hasExistingAdsAccount;
 	}
 
-	const accountCreationChecksResolved =
-		initHasExistingAdsAccountsRef.current !== null &&
-		initHasExistingMCAccountsRef.current !== null;
-
-	const shouldCreateAdsAccount =
-		initHasExistingAdsAccountsRef.current === false &&
-		initHasExistingMCAccountsRef.current === true;
+	const googleAdsAccountChecksResolved =
+		hasFinishedResolutionForExistingAdsAccount &&
+		hasFinishedResolutionForGoogleAdsAccount;
 
-	const shouldCreateMCAccount =
-		initHasExistingAdsAccountsRef.current === true &&
-		initHasExistingMCAccountsRef.current === false;
+	const googleMCAccountChecksResolved =
+		hasFinishedResolutionForGoogleMCAccount &&
+		hasFinishedResolutionForExistingMCAccounts;
 
-	const shouldCreateBothAccounts =
-		! initHasExistingAdsAccountsRef.current &&
-		! initHasExistingMCAccountsRef.current;
+	const accountCreationChecksResolved =
+		googleAdsAccountChecksResolved && googleMCAccountChecksResolved;
 
 	const isCreatingAccounts = !! isCreatingWhichAccount;
 
+	if ( googleAdsAccountChecksResolved && googleMCAccountChecksResolved ) {
+		if ( ! hasExistingAdsAccount || ! hasExistingMCAccount ) {
+			// Based on which accounts to create, set shouldCreateAccounts to 'ads, 'mc', or 'both'.
+			const createBothAccounts =
+				! hasExistingAdsAccount && ! hasExistingMCAccount;
+
+			if ( createBothAccounts ) {
+				shouldCreateAccounts.current = CREATING_BOTH_ACCOUNTS;
+			} else if ( ! hasExistingAdsAccount ) {
+				shouldCreateAccounts.current = CREATING_ADS_ACCOUNT;
+			} else {
+				shouldCreateAccounts.current = CREATING_MC_ACCOUNT;
+			}
+		}
+	}
+
 	useEffect( () => {
 		if ( ! response && loading ) {
 			return;
@@ -108,6 +120,7 @@ const useAutoCreateAdsMCAccounts = () => {
 				isCreatingWhichAccount: null,
 				accountsCreated: true,
 			} ) );
+			shouldCreateAccounts.current = null;
 			return;
 		}
 
@@ -124,6 +137,7 @@ const useAutoCreateAdsMCAccounts = () => {
 				isCreatingWhichAccount: null,
 				accountsCreated: true,
 			} ) );
+			shouldCreateAccounts.current = null;
 			return;
 		}
 
@@ -137,6 +151,7 @@ const useAutoCreateAdsMCAccounts = () => {
 				isCreatingWhichAccount: null,
 				accountsCreated: true,
 			} ) );
+			shouldCreateAccounts.current = null;
 		}
 	}, [ response, loading, isCreatingWhichAccount ] );
 
@@ -150,43 +165,33 @@ const useAutoCreateAdsMCAccounts = () => {
 				return;
 			}
 
-			if ( shouldCreateAdsAccount ) {
+			if ( shouldCreateAccounts.current ) {
 				setAccountsState( ( prevState ) => ( {
 					...prevState,
-					isCreatingWhichAccount: CREATING_ADS_ACCOUNT,
+					isCreatingWhichAccount: shouldCreateAccounts.current,
 				} ) );
-				await upsertAdsAccount();
-				return;
-			}
 
-			if ( shouldCreateMCAccount ) {
-				setAccountsState( ( prevState ) => ( {
-					...prevState,
-					isCreatingWhichAccount: CREATING_MC_ACCOUNT,
-				} ) );
-				await handleCreateAccount();
-				return;
-			}
+				if ( shouldCreateAccounts.current === CREATING_BOTH_ACCOUNTS ) {
+					await handleCreateAccount();
+					await upsertAdsAccount();
+					return;
+				}
+
+				if ( shouldCreateAccounts.current === CREATING_ADS_ACCOUNT ) {
+					await upsertAdsAccount();
+					return;
+				}
 
-			if ( shouldCreateBothAccounts ) {
-				setAccountsState( ( prevState ) => ( {
-					...prevState,
-					isCreatingWhichAccount: CREATING_BOTH_ACCOUNTS,
-				} ) );
 				await handleCreateAccount();
-				await upsertAdsAccount();
 			}
 		};
 
 		handleCreation();
 	}, [
 		accountCreationChecksResolved,
+		isCreatingAccounts,
 		accountsCreated,
 		handleCreateAccount,
-		isCreatingAccounts,
-		shouldCreateAdsAccount,
-		shouldCreateBothAccounts,
-		shouldCreateMCAccount,
 		upsertAdsAccount,
 	] );
 

diff --git a/js/src/hooks/useAutoCreateAdsMCAccounts.test.js b/js/src/hooks/useAutoCreateAdsMCAccounts.test.js
@@ -104,4 +104,49 @@ describe( 'useAutoCreateAdsMCAccounts hook', () => {
 			expect( result.current.isCreatingAccounts ).toBe( true );
 		} );
 	} );
+
+	describe( 'Existing accounts', () => {
+		beforeEach( () => {
+			useCreateMCAccount.mockReturnValue( [
+				handleCreateAccount,
+				{ response: { status: 200 } },
+			] );
+			useUpsertAdsAccount.mockReturnValue( [
+				upsertAdsAccount,
+				{ loading: false },
+			] );
+		} );
+
+		it( 'should not create accounts if they already exist', () => {
+			useExistingGoogleMCAccounts.mockReturnValue( {
+				data: [
+					{
+						id: 1,
+						name: 'Test MC Account',
+					},
+				],
+				hasFinishedResolution: true,
+			} );
+
+			useExistingGoogleAdsAccounts.mockReturnValue( {
+				existingAccounts: [
+					{
+						id: 1,
+						name: 'Test Ads Account',
+					},
+				],
+				hasFinishedResolution: true,
+			} );
+
+			const { result } = renderHook( () => useAutoCreateAdsMCAccounts() );
+
+			// It should not create any accounts.
+			expect( result.current.isCreatingWhichAccount ).toBe( null );
+			expect( result.current.isCreatingAccounts ).toBe( false );
+
+			// make sure functions are not called.
+			expect( handleCreateAccount ).not.toHaveBeenCalled();
+			expect( upsertAdsAccount ).not.toHaveBeenCalled();
+		} );
+	} );
 } );