From 9ae61cdb1b5fc8d578e51861362036d92187d4e0 Mon Sep 17 00:00:00 2001 From: Emanuele Buccelli Date: Wed, 27 Nov 2024 14:30:24 +0100 Subject: [PATCH] Stepper Tracking: Remove checks that prevent step start tracking (#96742) * No more checks * record - is_reentering_step_after_signup_complete and completed step/flow name also on step complete cleanup * Fix unit tests --------- Co-authored-by: Christos --- .../hooks/use-step-route-tracking/index.tsx | 58 +++++++++++-------- .../components/step-route/test/index.tsx | 35 +++++++---- 2 files changed, 58 insertions(+), 35 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/components/step-route/hooks/use-step-route-tracking/index.tsx b/client/landing/stepper/declarative-flow/internals/components/step-route/hooks/use-step-route-tracking/index.tsx index 404af6ddab2ed..2adf6feda575d 100644 --- a/client/landing/stepper/declarative-flow/internals/components/step-route/hooks/use-step-route-tracking/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/components/step-route/hooks/use-step-route-tracking/index.tsx @@ -80,38 +80,49 @@ export const useStepRouteTracking = ( { flow, stepSlug, skipStepRender }: Props const signupCompleteFlowName = getSignupCompleteFlowNameAndClear(); const signupCompleteStepName = getSignupCompleteStepNameAndClear(); - - const isReEnteringStep = + const isReEnteringStepAfterSignupComplete = signupCompleteFlowName === flowName && signupCompleteStepName === stepSlug; - if ( ! isReEnteringStep ) { - recordStepStart( flowName, kebabCase( stepSlug ), { + const reenteringStepAfterSignupCompleteProps = { + ...( isReEnteringStepAfterSignupComplete && { + is_reentering_step_after_signup_complete: true, + } ), + ...( signupCompleteFlowName && { signup_complete_flow_name: signupCompleteFlowName } ), + ...( signupCompleteStepName && { signup_complete_step_name: signupCompleteStepName } ), + }; + + recordStepStart( flowName, kebabCase( stepSlug ), { + intent, + is_in_hosting_flow: isAnyHostingFlow( flowName ), + ...( design && { assembler_source: getAssemblerSource( design ) } ), + ...( flowVariantSlug && { flow_variant: flowVariantSlug } ), + ...( skipStepRender && { skip_step_render: skipStepRender } ), + ...reenteringStepAfterSignupCompleteProps, + ...signupStepStartProps, + } ); + + // 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, + ...( skipStepRender && { skip_step_render: skipStepRender } ), + ...reenteringStepAfterSignupCompleteProps, + }, + }; + + const stepOldSlug = getStepOldSlug( stepSlug ); + if ( stepOldSlug ) { + recordStepStart( flowName, kebabCase( stepOldSlug ), { intent, is_in_hosting_flow: isAnyHostingFlow( flowName ), ...( design && { assembler_source: getAssemblerSource( design ) } ), ...( flowVariantSlug && { flow_variant: flowVariantSlug } ), ...( skipStepRender && { skip_step_render: skipStepRender } ), + ...reenteringStepAfterSignupCompleteProps, ...signupStepStartProps, } ); - - // 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, ...( skipStepRender && { skip_step_render: skipStepRender } ) }, - }; - - const stepOldSlug = getStepOldSlug( stepSlug ); - if ( stepOldSlug ) { - recordStepStart( flowName, kebabCase( stepOldSlug ), { - intent, - is_in_hosting_flow: isAnyHostingFlow( flowName ), - ...( design && { assembler_source: getAssemblerSource( design ) } ), - ...( flowVariantSlug && { flow_variant: flowVariantSlug } ), - ...( skipStepRender && { skip_step_render: skipStepRender } ), - ...signupStepStartProps, - } ); - } } // Also record page view for data and analytics @@ -119,6 +130,7 @@ export const useStepRouteTracking = ( { flow, stepSlug, skipStepRender }: Props const params = { flow: flowName, ...( skipStepRender && { skip_step_render: skipStepRender } ), + ...reenteringStepAfterSignupCompleteProps, }; recordPageView( pathname, pageTitle, params ); diff --git a/client/landing/stepper/declarative-flow/internals/components/step-route/test/index.tsx b/client/landing/stepper/declarative-flow/internals/components/step-route/test/index.tsx index 354c80dbed7ab..466fd5833bbc2 100644 --- a/client/landing/stepper/declarative-flow/internals/components/step-route/test/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/components/step-route/test/index.tsx @@ -170,13 +170,20 @@ describe( 'StepRoute', () => { } ); } ); - it( 'skips tracking when the step is re-entered', () => { + it( 'records recordStepStart with additional props when the step is re-entered', () => { ( getSignupCompleteFlowNameAndClear as jest.Mock ).mockReturnValue( 'some-flow' ); ( getSignupCompleteStepNameAndClear as jest.Mock ).mockReturnValue( 'some-step-slug' ); render( { step: regularStep } ); - expect( recordStepStart ).not.toHaveBeenCalled(); + expect( recordStepStart ).toHaveBeenCalledWith( 'some-flow', 'some-step-slug', { + is_reentering_step_after_signup_complete: true, + signup_complete_flow_name: 'some-flow', + signup_complete_step_name: 'some-step-slug', + intent: 'build', + assembler_source: 'premium', + is_in_hosting_flow: false, + } ); } ); it( 'records step-complete when the step is unmounted and step-start was previously recorded', () => { @@ -191,20 +198,12 @@ describe( 'StepRoute', () => { flow: 'some-flow', optionalProps: { intent: 'build', + signup_complete_flow_name: 'some-other-flow', + signup_complete_step_name: 'some-other-step-slug', }, } ); } ); - 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 } ); - - expect( recordStepStart ).not.toHaveBeenCalled(); - 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' ); @@ -217,10 +216,14 @@ describe( 'StepRoute', () => { assembler_source: 'premium', is_in_hosting_flow: false, skip_step_render: true, + signup_complete_flow_name: 'some-other-flow', + signup_complete_step_name: 'some-other-step-slug', } ); expect( recordPageView ).toHaveBeenCalledWith( '/', 'Setup > some-flow > some-step-slug', { flow: 'some-flow', skip_step_render: true, + signup_complete_flow_name: 'some-other-flow', + signup_complete_step_name: 'some-other-step-slug', } ); unmount(); @@ -231,6 +234,8 @@ describe( 'StepRoute', () => { optionalProps: { intent: 'build', skip_step_render: true, + signup_complete_flow_name: 'some-other-flow', + signup_complete_step_name: 'some-other-step-slug', }, } ); } ); @@ -245,10 +250,14 @@ describe( 'StepRoute', () => { assembler_source: 'premium', is_in_hosting_flow: false, skip_step_render: true, + signup_complete_flow_name: 'some-other-flow', + signup_complete_step_name: 'some-other-step-slug', } ); expect( recordPageView ).toHaveBeenCalledWith( '/', 'Setup > some-flow > some-step-slug', { flow: 'some-flow', skip_step_render: true, + signup_complete_flow_name: 'some-other-flow', + signup_complete_step_name: 'some-other-step-slug', } ); unmount(); @@ -259,6 +268,8 @@ describe( 'StepRoute', () => { optionalProps: { intent: 'build', skip_step_render: true, + signup_complete_flow_name: 'some-other-flow', + signup_complete_step_name: 'some-other-step-slug', }, } ); } );