Stepper: Track step-route unconditionally - logged-out/empty (#94817)

client/landing/stepper/declarative-flow/internals/analytics/record-step-complete.ts

@@ -5,7 +5,7 @@ import { STEPPER_TRACKS_EVENT_STEP_COMPLETE } from 'calypso/landing/stepper/cons
 export interface RecordStepCompleteProps {
 	flow: string;
 	step: string;
-	optionalProps?: Record< string, string | number | null >;
+	optionalProps?: Record< string, string | number | null | boolean >;
 }
 
 const recordStepComplete = ( { flow, step, optionalProps }: RecordStepCompleteProps ) => {

client/landing/stepper/declarative-flow/internals/components/step-route/hooks/use-step-route-tracking/index.tsx

@@ -1,6 +1,5 @@
 //* This hook is used to track the step route in the declarative flow.
 
-import { SiteDetails } from '@automattic/data-stores';
 import { isAnyHostingFlow } from '@automattic/onboarding';
 import { useEffect, useRef } from '@wordpress/element';
 import { getStepOldSlug } from 'calypso/landing/stepper/declarative-flow/helpers/get-step-old-slug';
@@ -21,18 +20,25 @@ import {
 import { useSelector } from 'calypso/state';
 import { isRequestingSite } from 'calypso/state/sites/selectors';
 
-// Ensure that the selected site is fetched, if available. This is used for event tracking purposes.
-// See https://github.com/Automattic/wp-calypso/pull/82981.
-const useIsRequestingSelectedSite = ( siteSlugOrId: string | number, site: SiteDetails | null ) => {
-	return useSelector( ( state ) => site && isRequestingSite( state, siteSlugOrId ) );
+/**
+ * We wait for the site to be fetched before tracking the step route when a site ID/slug are defined in the params.
+ * This is to ensure that the site data is available for event tracking purposes.
+ * See `superProps`, `site_plan_id`, and https://github.com/Automattic/wp-calypso/pull/82981.
+ */
+const useHasRequestedSelectedSite = () => {
+	const { site, siteSlugOrId } = useSiteData();
+	const isRequestingSelectedSite = useSelector(
+		( state ) => site && isRequestingSite( state, siteSlugOrId )
+	);
+
+	return siteSlugOrId ? !! site && ! isRequestingSelectedSite : true;
 };
 
 interface Props {
 	flowName: string;
 	stepSlug: string;
-	// If true, the tracking event will not be recorded
-	skipTracking: boolean;
 	flowVariantSlug?: string;
+	skipStepRender?: boolean;
 }
 
 /**
@@ -41,15 +47,14 @@ interface Props {
 export const useStepRouteTracking = ( {
 	flowName,
 	stepSlug,
-	skipTracking,
 	flowVariantSlug,
+	skipStepRender,
 }: Props ) => {
 	const intent = useIntent();
 	const design = useSelectedDesign();
-	const { site, siteSlugOrId } = useSiteData();
-	const isRequestingSelectedSite = useIsRequestingSelectedSite( siteSlugOrId, site );
-	const hasRequestedSelectedSite = siteSlugOrId ? !! site && ! isRequestingSelectedSite : true;
+	const hasRequestedSelectedSite = useHasRequestedSelectedSite();
 	const stepCompleteEventPropsRef = useRef< RecordStepCompleteProps | null >( null );
+	const pathname = window.location.pathname;
 
 	/**
 	 * Cleanup effect to record step-complete event when `StepRoute` unmounts.
@@ -66,8 +71,8 @@ export const useStepRouteTracking = ( {
 	}, [] );
 
 	useEffect( () => {
-		// We record the event only when the step is not empty. Additionally, we should not fire this event whenever the intent is changed
-		if ( ! hasRequestedSelectedSite || skipTracking ) {
+		// We wait for the site to be fetched before tracking the step route.
+		if ( ! hasRequestedSelectedSite ) {
 			return;
 		}
 
@@ -83,13 +88,14 @@ export const useStepRouteTracking = ( {
 				is_in_hosting_flow: isAnyHostingFlow( flowName ),
 				...( design && { assembler_source: getAssemblerSource( design ) } ),
 				...( flowVariantSlug && { flow_variant: flowVariantSlug } ),
+				...( skipStepRender && { skip_step_render: skipStepRender } ),
 			} );
 
 			// Apply the props to record in the exit/step-complete event. We only record this if start event gets recorded.
 			stepCompleteEventPropsRef.current = {
 				flow: flowName,
 				step: stepSlug,
-				optionalProps: { intent },
+				optionalProps: { intent, ...( skipStepRender && { skip_step_render: skipStepRender } ) },
 			};
 
 			const stepOldSlug = getStepOldSlug( stepSlug );
@@ -100,20 +106,23 @@ export const useStepRouteTracking = ( {
 					is_in_hosting_flow: isAnyHostingFlow( flowName ),
 					...( design && { assembler_source: getAssemblerSource( design ) } ),
 					...( flowVariantSlug && { flow_variant: flowVariantSlug } ),
+					...( skipStepRender && { skip_step_render: skipStepRender } ),
 				} );
 			}
 		}
 
 		// Also record page view for data and analytics
-		const pathname = window.location.pathname;
 		const pageTitle = `Setup > ${ flowName } > ${ stepSlug }`;
 		const params = {
 			flow: flowName,
+			...( skipStepRender && { skip_step_render: skipStepRender } ),
 		};
 		recordPageView( pathname, pageTitle, params );
 
 		// We leave out intent and design from the dependency list, due to the ONBOARD_STORE being reset in the exit flow.
 		// The store reset causes these values to become empty, and may trigger this event again.
+		// We also leave out pathname. The respective event (calypso_page_view) is recorded behind a timeout and we don't want to trigger it again.
+		//     - window.location.pathname called inside the effect keeps referring to the previous path on a redirect. So we moved it outside.
 		// eslint-disable-next-line react-hooks/exhaustive-deps
-	}, [ flowName, hasRequestedSelectedSite, stepSlug, skipTracking ] );
+	}, [ flowName, hasRequestedSelectedSite, stepSlug, skipStepRender ] );
 };

client/landing/stepper/declarative-flow/internals/components/step-route/index.tsx

@@ -25,15 +25,14 @@ const StepRoute = ( { step, flow, showWooLogo, renderStep, navigate }: StepRoute
 	const loginUrl = useLoginUrlForFlow( { flow } );
 	const shouldAuthUser = step.requiresLoggedInUser && ! userIsLoggedIn;
 	const shouldSkipRender = shouldAuthUser || ! stepContent;
-	const skipTracking = shouldAuthUser || ! stepContent;
 
 	const useBuiltItInAuth = flow.__experimentalUseBuiltinAuth;
 
 	useStepRouteTracking( {
 		flowName: flow.name,
 		stepSlug: step.slug,
-		skipTracking,
 		flowVariantSlug: flow.variantSlug,
+		skipStepRender: shouldSkipRender,
 	} );
 
 	useEffect( () => {

client/landing/stepper/declarative-flow/internals/components/step-route/test/index.tsx

@@ -152,15 +152,15 @@ describe( 'StepRoute', () => {
 	} );
 
 	describe( 'tracking', () => {
-		it( 'records a page view when the step is rendered', async () => {
+		it( 'records a page view', async () => {
 			render( { step: regularStep } );
 
 			expect( recordPageView ).toHaveBeenCalledWith( '/', 'Setup > some-flow > some-step-slug', {
 				flow: 'some-flow',
 			} );
 		} );
 
-		it( 'records recordStepStart when the step is rendered', async () => {
+		it( 'records recordStepStart', async () => {
 			render( { step: regularStep } );
 
 			expect( recordStepStart ).toHaveBeenCalledWith( 'some-flow', 'some-step-slug', {
@@ -170,14 +170,6 @@ describe( 'StepRoute', () => {
 			} );
 		} );
 
-		it( 'does not record start and page view when the login is required and the user is not logged in', async () => {
-			( isUserLoggedIn as jest.Mock ).mockReturnValue( false );
-			render( { step: requiresLoginStep } );
-
-			expect( recordStepStart ).not.toHaveBeenCalled();
-			expect( recordPageView ).not.toHaveBeenCalled();
-		} );
-
 		it( 'skips tracking when the step is re-entered', () => {
 			( getSignupCompleteFlowNameAndClear as jest.Mock ).mockReturnValue( 'some-flow' );
 			( getSignupCompleteStepNameAndClear as jest.Mock ).mockReturnValue( 'some-step-slug' );
@@ -187,15 +179,7 @@ describe( 'StepRoute', () => {
 			expect( recordStepStart ).not.toHaveBeenCalled();
 		} );
 
-		it( 'skips trackings when the renderStep returns null', () => {
-			render( { step: regularStep, renderStep: () => null } );
-
-			expect( recordStepStart ).not.toHaveBeenCalled();
-			expect( recordPageView ).not.toHaveBeenCalled();
-		} );
-
-		it( 'tracks step-complete when the step is unmounted and step-start was previously recorded', () => {
-			( isUserLoggedIn as jest.Mock ).mockReturnValue( true );
+		it( 'records step-complete when the step is unmounted and step-start was previously recorded', () => {
 			( getSignupCompleteFlowNameAndClear as jest.Mock ).mockReturnValue( 'some-other-flow' );
 			( getSignupCompleteStepNameAndClear as jest.Mock ).mockReturnValue( 'some-other-step-slug' );
 			const { unmount } = render( { step: regularStep } );
@@ -211,7 +195,7 @@ describe( 'StepRoute', () => {
 			} );
 		} );
 
-		it( 'skips tracking step-complete when the step is unmounted and step-start was not recorded', () => {
+		it( 'skips recording step-complete when the step is unmounted and step-start was not recorded', () => {
 			( getSignupCompleteFlowNameAndClear as jest.Mock ).mockReturnValue( 'some-flow' );
 			( getSignupCompleteStepNameAndClear as jest.Mock ).mockReturnValue( 'some-step-slug' );
 			const { unmount } = render( { step: regularStep } );
@@ -220,5 +204,63 @@ describe( 'StepRoute', () => {
 			unmount();
 			expect( recordStepComplete ).not.toHaveBeenCalled();
 		} );
+
+		it( 'records skip_step_render on start, complete and page view when the login is required and the user is not logged in', async () => {
+			( isUserLoggedIn as jest.Mock ).mockReturnValue( false );
+			( getSignupCompleteFlowNameAndClear as jest.Mock ).mockReturnValue( 'some-other-flow' );
+			( getSignupCompleteStepNameAndClear as jest.Mock ).mockReturnValue( 'some-other-step-slug' );
+
+			const { unmount } = render( { step: requiresLoginStep } );
+
+			expect( recordStepStart ).toHaveBeenCalledWith( 'some-flow', 'some-step-slug', {
+				intent: 'build',
+				assembler_source: 'premium',
+				is_in_hosting_flow: false,
+				skip_step_render: true,
+			} );
+			expect( recordPageView ).toHaveBeenCalledWith( '/', 'Setup > some-flow > some-step-slug', {
+				flow: 'some-flow',
+				skip_step_render: true,
+			} );
+
+			unmount();
+
+			expect( recordStepComplete ).toHaveBeenCalledWith( {
+				step: 'some-step-slug',
+				flow: 'some-flow',
+				optionalProps: {
+					intent: 'build',
+					skip_step_render: true,
+				},
+			} );
+		} );
+
+		it( 'records skip_step_render on start, complete and page view when renderStep returns null', async () => {
+			( getSignupCompleteFlowNameAndClear as jest.Mock ).mockReturnValue( 'some-other-flow' );
+			( getSignupCompleteStepNameAndClear as jest.Mock ).mockReturnValue( 'some-other-step-slug' );
+			const { unmount } = render( { step: regularStep, renderStep: () => null } );
+
+			expect( recordStepStart ).toHaveBeenCalledWith( 'some-flow', 'some-step-slug', {
+				intent: 'build',
+				assembler_source: 'premium',
+				is_in_hosting_flow: false,
+				skip_step_render: true,
+			} );
+			expect( recordPageView ).toHaveBeenCalledWith( '/', 'Setup > some-flow > some-step-slug', {
+				flow: 'some-flow',
+				skip_step_render: true,
+			} );
+
+			unmount();
+
+			expect( recordStepComplete ).toHaveBeenCalledWith( {
+				step: 'some-step-slug',
+				flow: 'some-flow',
+				optionalProps: {
+					intent: 'build',
+					skip_step_render: true,
+				},
+			} );
+		} );
 	} );
 } );