Merge pull request #12084 from google/enhancement/11960-fix-connect-analytics-banner

Update PUE Analytics notice to resume setup and add external link icon.

Author: tofumatt

assets/js/components/email-reporting/notices/SetupAnalyticsNotice.js

@@ -37,6 +37,7 @@ import { CORE_SITE } from '@/js/googlesitekit/datastore/site/constants';
 import { CORE_MODULES } from '@/js/googlesitekit/modules/datastore/constants';
 import { MODULE_SLUG_ANALYTICS_4 } from '@/js/modules/analytics-4/constants';
 import useActivateModuleCallback from '@/js/hooks/useActivateModuleCallback';
+import useCompleteModuleActivationCallback from '@/js/hooks/useCompleteModuleActivationCallback';
 import Link from '@/js/components/Link';
 import useViewOnly from '@/js/hooks/useViewOnly';
 
@@ -52,6 +53,10 @@ export default function SetupAnalyticsNotice() {
 
 	const isViewOnly = useViewOnly();
 
+	const isAnalyticsActive = useSelect( ( select ) =>
+		select( CORE_MODULES ).isModuleActive( MODULE_SLUG_ANALYTICS_4 )
+	);
+
 	const isAnalyticsConnected = useSelect( ( select ) =>
 		select( CORE_MODULES ).isModuleConnected( MODULE_SLUG_ANALYTICS_4 )
 	);
@@ -72,10 +77,23 @@ export default function SetupAnalyticsNotice() {
 		MODULE_SLUG_ANALYTICS_4
 	);
 
+	const completeModuleActivation = useCompleteModuleActivationCallback(
+		MODULE_SLUG_ANALYTICS_4
+	);
+
+	// If Analytics is already active but not connected, skip activation
+	// and go directly to the setup flow.
+	const onClickCallback = isAnalyticsActive
+		? completeModuleActivation
+		: activateAnalytics;
+
 	const handleCTAClick = useCallback( () => {
+		if ( ! onClickCallback ) {
+			return;
+		}
 		setInProgress( true );
-		activateAnalytics();
-	}, [ activateAnalytics ] );
+		onClickCallback();
+	}, [ onClickCallback ] );
 
 	const handleDismiss = useCallback( async () => {
 		await dismissItem(
@@ -97,6 +115,10 @@ export default function SetupAnalyticsNotice() {
 		return null;
 	}
 
+	const ctaLabel = isAnalyticsActive
+		? __( 'Complete setup', 'google-site-kit' )
+		: __( 'Connect Analytics', 'google-site-kit' );
+
 	return (
 		<Notice
 			type={ TYPES.NEW }
@@ -110,17 +132,11 @@ export default function SetupAnalyticsNotice() {
 					'google-site-kit'
 				),
 				{
-					a: (
-						<Link
-							href={ learnMoreLink }
-							external
-							hideExternalIndicator
-						/>
-					),
+					a: <Link href={ learnMoreLink } external />,
 				}
 			) }
 			ctaButton={ {
-				label: __( 'Connect Analytics', 'google-site-kit' ),
+				label: ctaLabel,
 				inProgress,
 				disabled: inProgress,
 				onClick: handleCTAClick,

assets/js/components/email-reporting/notices/SetupAnalyticsNotice.test.js

@@ -19,7 +19,7 @@
 /**
  * External dependencies
  */
-import { waitFor } from '@testing-library/react';
+import { waitFor, act } from '@testing-library/react';
 
 /**
  * Internal dependencies
@@ -33,6 +33,7 @@ import {
 	provideUserAuthentication,
 	provideModuleRegistrations,
 	provideSiteInfo,
+	waitForDefaultTimeouts,
 } from '../../../../../tests/js/test-utils';
 import { CORE_USER } from '@/js/googlesitekit/datastore/user/constants';
 import { MODULE_SLUG_ANALYTICS_4 } from '@/js/modules/analytics-4/constants';
@@ -122,7 +123,29 @@ describe( 'SetupAnalyticsNotice', () => {
 		);
 	} );
 
-	it( 'dismisses the notice when "Got it" is clicked', async () => {
+	it( 'dismisses the notice when "Maybe later" is clicked', async () => {
+		// Create a fresh registry without module registrations to avoid
+		// async resolver state updates that cause act() warnings.
+		registry = createTestRegistry();
+		provideSiteInfo( registry );
+		provideUserAuthentication( registry );
+		provideUserCapabilities( registry );
+		provideModules( registry, [
+			{
+				slug: MODULE_SLUG_ANALYTICS_4,
+				active: false,
+				connected: false,
+				disconnectedAt: false,
+			},
+		] );
+
+		// Note: Intentionally not calling `provideModuleRegistrations` here
+		// to prevent the Analytics 4 store resolver from running.
+		registry.dispatch( CORE_USER ).receiveGetDismissedItems( [] );
+		registry.dispatch( CORE_SITE ).receiveGetEmailReportingSettings( {
+			enabled: true,
+		} );
+
 		fetchMock.getOnce( fetchGetDismissedItems, { body: [] } );
 		fetchMock.postOnce( fetchDismissItem, {
 			body: [ EMAIL_REPORTING_SETUP_ANALYTICS_NOTICE_DISMISSED_ITEM ],
@@ -171,4 +194,98 @@ describe( 'SetupAnalyticsNotice', () => {
 
 		expect( container ).toBeEmptyDOMElement();
 	} );
+
+	it( 'shows spinner and disabled state while the activation is in progress', async () => {
+		const moduleActivationEndpoint = RegExp(
+			'google-site-kit/v1/core/modules/data/activation'
+		);
+
+		const userAuthenticationEndpoint = RegExp(
+			'^/google-site-kit/v1/core/user/data/authentication'
+		);
+
+		fetchMock.getOnce( userAuthenticationEndpoint, {
+			body: { needsReauthentication: false },
+		} );
+
+		// Use a never-resolving promise to keep the activation in progress.
+		fetchMock.postOnce( moduleActivationEndpoint, new Promise( () => {} ) );
+
+		const { getByRole } = render( <SetupAnalyticsNotice />, {
+			registry,
+		} );
+
+		const ctaButton = getByRole( 'button', {
+			name: /connect analytics/i,
+		} );
+
+		fireEvent.click( ctaButton );
+
+		// Wait for the spinner to appear.
+		await waitFor( () => {
+			expect( ctaButton ).toHaveAttribute( 'disabled' );
+		} );
+
+		// Verify the button contains the spinner (via the CSS class).
+		expect( ctaButton ).toHaveClass(
+			'googlesitekit-notice__cta--spinner__running'
+		);
+	} );
+
+	it( 'shows "Complete setup" CTA and skips activation when Analytics is active but not connected', async () => {
+		// Create a fresh registry with Analytics active but not connected.
+		registry = createTestRegistry();
+		provideSiteInfo( registry );
+		provideUserAuthentication( registry );
+		provideUserCapabilities( registry );
+		provideModules( registry, [
+			{
+				slug: MODULE_SLUG_ANALYTICS_4,
+				active: true,
+				connected: false,
+				disconnectedAt: false,
+			},
+		] );
+		provideModuleRegistrations( registry );
+
+		registry.dispatch( CORE_USER ).receiveGetDismissedItems( [] );
+		registry.dispatch( CORE_SITE ).receiveGetEmailReportingSettings( {
+			enabled: true,
+		} );
+
+		const moduleActivationEndpoint = RegExp(
+			'google-site-kit/v1/core/modules/data/activation'
+		);
+
+		const { getByRole } = render( <SetupAnalyticsNotice />, {
+			registry,
+		} );
+
+		// Verify the CTA label is "Complete setup".
+		const ctaButton = getByRole( 'button', {
+			name: /complete setup/i,
+		} );
+
+		expect( ctaButton ).toBeInTheDocument();
+
+		await act( async () => {
+			fireEvent.click( ctaButton );
+			await waitForDefaultTimeouts();
+		} );
+
+		// Verify that the activation endpoint was NOT called.
+		expect( fetchMock ).not.toHaveFetched( moduleActivationEndpoint );
+	} );
+
+	it( 'shows external link indicator on the "Learn more" link', () => {
+		const { container } = render( <SetupAnalyticsNotice />, {
+			registry,
+		} );
+
+		// Find the link by looking for the external icon SVG.
+		const externalIcon = container.querySelector(
+			'.googlesitekit-cta-link svg'
+		);
+		expect( externalIcon ).toBeInTheDocument();
+	} );
 } );

assets/js/hooks/useCompleteModuleActivationCallback.js

@@ -46,12 +46,15 @@ export default function useCompleteModuleActivationCallback( moduleSlug ) {
 	const canManageOptions = useSelect( ( select ) =>
 		select( CORE_USER ).hasCapability( PERMISSION_MANAGE_OPTIONS )
 	);
-	const moduleStoreName = useSelect( ( select ) =>
-		select( CORE_MODULES ).getModuleStoreName( moduleSlug )
-	);
-	const adminReauthURL = useSelect( ( select ) =>
-		select( moduleStoreName )?.getAdminReauthURL()
-	);
+	const adminReauthURL = useSelect( ( select ) => {
+		const moduleStoreName =
+			select( CORE_MODULES ).getModuleStoreName( moduleSlug );
+
+		if ( ! moduleStoreName ) {
+			return undefined;
+		}
+		return select( moduleStoreName )?.getAdminReauthURL();
+	} );
 	const { navigateTo } = useDispatch( CORE_LOCATION );
 
 	const completeModuleActivationCallback = useCallback(

assets/js/modules/analytics-4/components/audience-segmentation/settings/SettingsCardVisitorGroups/index.test.js

@@ -158,10 +158,16 @@ describe( 'SettingsCardVisitorGroups', () => {
 					audienceSegmentationSetupCompletedBy: null,
 				} );
 
+			// Set available audiences directly to ensure the selector returns
+			// the data without triggering a network request during save.
+			registry
+				.dispatch( MODULES_ANALYTICS_4 )
+				.setAvailableAudiences( availableAudiences );
+
 			fetchMock.post( audienceSettingsEndpoint, ( url, opts ) => {
 				const { data } = JSON.parse( opts.body );
-				// Return the same settings passed to the API.
-				return { body: data, status: 200 };
+				// Return the settings object directly (not wrapped in data).
+				return { body: data.settings, status: 200 };
 			} );
 		} );