Fix consent mode Ads connection status logic

assets/js/components/settings/SettingsCardConsentMode.stories.js

@@ -50,6 +50,19 @@ export const WithAdsConnected = Template.bind( {} );
 WithAdsConnected.storyName = 'WithAdsConnected';
 WithAdsConnected.args = {
 	setupRegistry: ( registry ) => {
+		provideModules( registry, [
+			{
+				active: true,
+				connected: true,
+				slug: 'analytics-4',
+			},
+			{
+				active: true,
+				connected: true,
+				slug: 'ads',
+			},
+		] );
+
 		// Set consent mode to disabled in order to show the additional Ads related notice.
 		registry.dispatch( CORE_SITE ).setConsentModeEnabled( false );
 
@@ -149,8 +162,8 @@ export default {
 						slug: 'analytics-4',
 					},
 					{
-						active: true,
-						connected: true,
+						active: false,
+						connected: false,
 						slug: 'ads',
 					},
 				] );
@@ -163,10 +176,6 @@ export default {
 					.dispatch( MODULES_ANALYTICS_4 )
 					.receiveGetSettings( {} );
 
-				registry.dispatch( MODULES_ADS ).setSettings( {
-					conversionID: '',
-				} );
-
 				// Mock the consent mode endpoint to allow toggling the switch.
 				fetchMock.post(
 					RegExp( 'google-site-kit/v1/core/site/data/consent-mode' ),

assets/js/googlesitekit/datastore/site/consent-mode.js

@@ -23,7 +23,6 @@ import { createFetchStore } from '../../data/create-fetch-store';
 import { createReducer } from '../../data/create-reducer';
 import { CORE_MODULES } from '../../modules/datastore/constants';
 import { CORE_SITE } from './constants';
-import { MODULES_ADS } from '../../../modules/ads/datastore/constants';
 import { MODULES_ANALYTICS_4 } from '../../../modules/analytics-4/datastore/constants';
 import invariant from 'invariant';
 import { isPlainObject } from 'lodash';
@@ -180,42 +179,42 @@ const baseSelectors = {
 	isAdsConnected: createRegistrySelector( ( select ) => () => {
 		const { isModuleConnected } = select( CORE_MODULES );
 
-		if (
-			! isModuleConnected( 'analytics-4' ) ||
-			! isModuleConnected( 'ads' )
-		) {
-			return false;
-		}
-
-		const { getAdsLinked, getGoogleTagContainerDestinationIDs } =
-			select( MODULES_ANALYTICS_4 );
-		const { getConversionID } = select( MODULES_ADS );
-
-		const conversionID = getConversionID();
-		const adsLinked = getAdsLinked();
-		const googleTagContainerDestinationIDs =
-			getGoogleTagContainerDestinationIDs();
-
-		if (
-			[
-				conversionID,
-				adsLinked,
-				googleTagContainerDestinationIDs,
-			].includes( undefined )
-		) {
-			return undefined;
+		// The Ads module being connected implies that an Ads conversion tracking
+		// ID is set. If so, return true.
+		if ( isModuleConnected( 'ads' ) ) {
+			return true;
 		}
 
-		if (
-			Array.isArray( googleTagContainerDestinationIDs ) &&
-			googleTagContainerDestinationIDs.some( ( id ) =>
-				id.startsWith( 'AW-' )
-			)
-		) {
-			return true;
+		if ( isModuleConnected( 'analytics-4' ) ) {
+			const { getAdsLinked, getGoogleTagContainerDestinationIDs } =
+				select( MODULES_ANALYTICS_4 );
+
+			const adsLinked = getAdsLinked();
+			const googleTagContainerDestinationIDs =
+				getGoogleTagContainerDestinationIDs();
+
+			// If necessary settings have not loaded, return undefined.
+			if (
+				[ adsLinked, googleTagContainerDestinationIDs ].includes(
+					undefined
+				)
+			) {
+				return undefined;
+			}
+
+			if (
+				Array.isArray( googleTagContainerDestinationIDs ) &&
+				googleTagContainerDestinationIDs.some( ( id ) =>
+					id.startsWith( 'AW-' )
+				)
+			) {
+				return true;
+			}
+
+			return !! adsLinked;
 		}
 
-		return !! conversionID || !! adsLinked;
+		return false;
 	} ),
 };
 

assets/js/googlesitekit/datastore/site/consent-mode.test.js

@@ -27,7 +27,6 @@ import {
 } from '../../../../../tests/js/utils';
 import { CORE_SITE } from './constants';
 import { MODULES_ANALYTICS_4 } from '../../../modules/analytics-4/datastore/constants';
-import { MODULES_ADS } from '../../../modules/ads/datastore/constants';
 
 describe( 'core/site Consent Mode', () => {
 	let registry;
@@ -303,23 +302,15 @@ describe( 'core/site Consent Mode', () => {
 		} );
 
 		describe( 'isAdsConnected', () => {
-			it( 'returns false if the Analytics module is not connected', () => {
+			it( 'returns false if both the Ads and Analytics modules are disconnected', () => {
 				provideModules( registry, [
 					{
-						slug: 'analytics-4',
+						slug: 'ads',
 						active: false,
 						connected: false,
 					},
-				] );
-
-				expect( registry.select( CORE_SITE ).isAdsConnected() ).toBe(
-					false
-				);
-			} );
-			it( 'returns false if the Ads module is not connected', () => {
-				provideModules( registry, [
 					{
-						slug: 'ads',
+						slug: 'analytics-4',
 						active: false,
 						connected: false,
 					},
@@ -330,61 +321,40 @@ describe( 'core/site Consent Mode', () => {
 				);
 			} );
 
-			it( 'returns undefined if either the Ads conversion ID in Ads or the Analytics and Ads linked status is undefined', () => {
+			it( 'returns true if the Ads module is connected', () => {
 				provideModules( registry, [
-					{
-						slug: 'analytics-4',
-						active: true,
-						connected: true,
-					},
 					{
 						slug: 'ads',
 						active: true,
 						connected: true,
 					},
 				] );
 
-				registry.dispatch( MODULES_ADS ).receiveGetSettings( {} );
-
-				registry
-					.dispatch( MODULES_ANALYTICS_4 )
-					.receiveGetSettings( {} );
-
-				registry.dispatch( MODULES_ADS ).receiveGetSettings( {} );
-
 				expect( registry.select( CORE_SITE ).isAdsConnected() ).toBe(
-					undefined
+					true
 				);
 			} );
 
-			it( 'returns true if an Ads conversion ID is set in the Ads module', () => {
+			it( 'returns undefined if the Ads module is disconnected and the Analytics module settings have not loaded', () => {
 				provideModules( registry, [
 					{
-						slug: 'analytics-4',
-						active: true,
-						connected: true,
+						slug: 'ads',
+						active: false,
+						connected: false,
 					},
 					{
-						slug: 'ads',
+						slug: 'analytics-4',
 						active: true,
 						connected: true,
 					},
 				] );
 
-				registry.dispatch( MODULES_ADS ).receiveGetSettings( {
-					conversionID: 'AW-12345',
-				} );
-
-				registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetSettings( {
-					// Set the following to default, as otherwise if it is set to
-					// undefined, the `core/site` `isAdsConnected` selector will
-					// return undefined.
-					adsLinked: false,
-					googleTagContainerDestinationIDs: null,
-				} );
+				registry
+					.dispatch( MODULES_ANALYTICS_4 )
+					.receiveGetSettings( {} );
 
 				expect( registry.select( CORE_SITE ).isAdsConnected() ).toBe(
-					true
+					undefined
 				);
 			} );
 
@@ -395,19 +365,13 @@ describe( 'core/site Consent Mode', () => {
 						active: true,
 						connected: true,
 					},
-					{
-						slug: 'ads',
-						active: true,
-						connected: true,
-					},
 				] );
 
-				registry.dispatch( MODULES_ADS ).receiveGetSettings( {
-					conversionID: 'AW-12345',
-				} );
-
 				registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetSettings( {
 					adsLinked: true,
+					// Set the following to default, as otherwise if it is set to
+					// undefined, the `core/site` `isAdsConnected` selector will
+					// return undefined.
 					googleTagContainerDestinationIDs: null,
 				} );
 
@@ -423,23 +387,13 @@ describe( 'core/site Consent Mode', () => {
 						active: true,
 						connected: true,
 					},
-					{
-						slug: 'ads',
-						active: true,
-						connected: true,
-					},
 				] );
 
-				registry.dispatch( MODULES_ADS ).receiveGetSettings( {
-					conversionID: 'AW-12345',
-				} );
-
 				registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetSettings( {
 					googleTagContainerDestinationIDs: [ 'AW-12345' ],
 					// Set the following to default, as otherwise if it is set to
 					// undefined, the `core/site` `isAdsConnected` selector will
 					// return undefined.
-					conversionID: '',
 					adsLinked: false,
 				} );
 
@@ -448,26 +402,21 @@ describe( 'core/site Consent Mode', () => {
 				);
 			} );
 
-			it( 'returns false if neither an Ads conversion ID is set in Ads, nor Analytics and Ads are linked', () => {
+			it( 'returns false if the Ads module is disconnected, Analytics and Ads are not linked, and the Google Tag does not have an Ads conversion tracking ID as a destination', () => {
 				provideModules( registry, [
-					{
-						slug: 'analytics-4',
-						active: true,
-						connected: true,
-					},
 					{
 						slug: 'ads',
 						active: false,
 						connected: false,
 					},
+					{
+						slug: 'analytics-4',
+						active: true,
+						connected: true,
+					},
 				] );
 
-				registry.dispatch( MODULES_ADS ).receiveGetSettings( {
-					conversionID: '',
-				} );
-
 				registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetSettings( {
-					conversionID: '',
 					adsLinked: false,
 					googleTagContainerDestinationIDs: null,
 				} );