From 72d2f2428303245050f990524645edbe648d29da Mon Sep 17 00:00:00 2001 From: Christos Date: Tue, 17 Sep 2024 14:22:03 +0300 Subject: [PATCH] Stepper: Record calypso_signup_start unconditionally => multiple logs --- .../use-sign-up-start-tracking/index.tsx | 31 +++------ .../use-sign-up-start-tracking/test/index.tsx | 65 ++----------------- .../declarative-flow/internals/index.tsx | 2 +- 3 files changed, 13 insertions(+), 85 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/index.tsx b/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/index.tsx index 04e192dd54ee1..2318ada9766c5 100644 --- a/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/index.tsx @@ -1,5 +1,4 @@ -import { SENSEI_FLOW } from '@automattic/onboarding'; -import { useCallback, useEffect, useMemo } from 'react'; +import { useEffect, useMemo } from 'react'; import { useSearchParams } from 'react-router-dom'; import { recordSignupStart } from 'calypso/lib/analytics/signup'; import { type Flow } from '../../types'; @@ -9,22 +8,15 @@ import { type Flow } from '../../types'; */ interface Props { flow: Flow; - currentStepRoute: string; } -export const useSignUpStartTracking = ( { flow, currentStepRoute }: Props ) => { - const steps = flow.useSteps(); - const [ queryParams, setQuery ] = useSearchParams(); +export const useSignUpStartTracking = ( { flow }: Props ) => { + const [ queryParams ] = useSearchParams(); const ref = queryParams.get( 'ref' ) || ''; - - // TODO: Using the new start flag we can remove reference to SENSEI_FLOW - const firstStepSlug = ( flow.name === SENSEI_FLOW ? steps[ 1 ] : steps[ 0 ] ).slug; - const isFirstStep = firstStepSlug === currentStepRoute; const flowVariant = flow.variantSlug; - const signupStartEventProps = flow.useSignupStartEventProps?.(); - const isStartingFlow = isFirstStep || queryParams.has( 'start' ); const flowName = flow.name; - const shouldTrack = flow.isSignupFlow && isStartingFlow; + const isSignupFlow = flow.isSignupFlow; + const signupStartEventProps = flow.useSignupStartEventProps?.(); const extraProps = useMemo( () => ( { @@ -34,18 +26,11 @@ export const useSignUpStartTracking = ( { flow, currentStepRoute }: Props ) => { [ signupStartEventProps, flowVariant ] ); - const removeSignupParam = useCallback( () => { - if ( queryParams.has( 'start' ) ) { - queryParams.delete( 'start' ); - setQuery( queryParams ); - } - }, [ queryParams, setQuery ] ); - useEffect( () => { - if ( ! shouldTrack ) { + if ( ! isSignupFlow ) { return; } + recordSignupStart( flowName, ref, extraProps || {} ); - removeSignupParam(); - }, [ extraProps, flowName, ref, removeSignupParam, shouldTrack ] ); + }, [ extraProps, flowName, ref, isSignupFlow ] ); }; diff --git a/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/test/index.tsx b/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/test/index.tsx index 6b61c011b3892..5cf1282150d23 100644 --- a/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/test/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/test/index.tsx @@ -3,7 +3,6 @@ */ import { recordTracksEvent } from '@automattic/calypso-analytics'; -import { SENSEI_FLOW } from '@automattic/onboarding'; import { addQueryArgs } from '@wordpress/url'; import React from 'react'; import { MemoryRouter } from 'react-router-dom'; @@ -28,21 +27,13 @@ const signUpFlow: Flow = { isSignupFlow: true, }; -const senseiFlow: Flow = { - ...regularFlow, - name: SENSEI_FLOW, - // The original sensei flow is missing the isSignupFlow flag as true, it will be addressed by wp-calypso/pull/91593 - isSignupFlow: true, -}; - jest.mock( '@automattic/calypso-analytics' ); -const render = ( { flow, currentStepRoute, queryParams = {} } ) => { +const render = ( { flow, queryParams = {} } ) => { return renderHookWithProvider( () => useSignUpStartTracking( { flow, - currentStepRoute, } ), { wrapper: ( { children } ) => ( @@ -60,22 +51,21 @@ describe( 'useSignUpTracking', () => { } ); it( 'does not track event when the flow is not a isSignupFlow', () => { - render( { flow: regularFlow, currentStepRoute: 'step-1' } ); + render( { flow: regularFlow } ); expect( recordTracksEvent ).not.toHaveBeenCalled(); } ); it( 'does not track event when the flow is not a isSignupFlow and the signup flag is set', () => { - render( { flow: regularFlow, currentStepRoute: 'step-1', queryParams: { start: 1 } } ); + render( { flow: regularFlow, queryParams: { start: 1 } } ); expect( recordTracksEvent ).not.toHaveBeenCalled(); } ); describe( 'sign-up-flow', () => { - it( 'tracks the event current step is first step', () => { + it( 'tracks the event current', () => { render( { flow: signUpFlow, - currentStepRoute: 'step-1', queryParams: { ref: 'another-flow-or-cta' }, } ); @@ -85,26 +75,12 @@ describe( 'useSignUpTracking', () => { } ); } ); - it( 'tracks the event when the step is not the first but the start flag is set', () => { - render( { - flow: signUpFlow, - currentStepRoute: 'step-2', - queryParams: { start: 1 }, - } ); - - expect( recordTracksEvent ).toHaveBeenCalledWith( 'calypso_signup_start', { - flow: 'sign-up-flow', - ref: '', - } ); - } ); - it( 'tracks the event with extra props from useSighupStartEventProps', () => { render( { flow: { ...signUpFlow, useSignupStartEventProps: () => ( { extra: 'props' } ), } satisfies Flow, - currentStepRoute: 'step-1', queryParams: { ref: 'another-flow-or-cta' }, } ); @@ -121,7 +97,6 @@ describe( 'useSignUpTracking', () => { ...signUpFlow, variantSlug: 'variant-slug', } satisfies Flow, - currentStepRoute: 'step-1', } ); expect( recordTracksEvent ).toHaveBeenCalledWith( 'calypso_signup_start', { @@ -130,37 +105,5 @@ describe( 'useSignUpTracking', () => { ref: '', } ); } ); - - it( 'does not track events current step is NOT the first step', () => { - render( { flow: signUpFlow, currentStepRoute: 'step-2' } ); - - expect( recordTracksEvent ).not.toHaveBeenCalled(); - } ); - - // Check if sensei is a sign-up flow; - it( "tracks when the user is on the sensei's flow second step", () => { - render( { flow: senseiFlow, currentStepRoute: 'step-2' } ); - - expect( recordTracksEvent ).toHaveBeenCalledWith( 'calypso_signup_start', { - flow: SENSEI_FLOW, - ref: '', - } ); - } ); - - it( 'does not trigger the event on rerender', () => { - const { rerender } = render( { - flow: { ...signUpFlow, useSignupStartEventProps: () => ( { extra: 'props' } ) }, - currentStepRoute: 'step-1', - queryParams: { ref: 'another-flow-or-cta' }, - } ); - - rerender(); - - expect( recordTracksEvent ).toHaveBeenNthCalledWith( 1, 'calypso_signup_start', { - flow: 'sign-up-flow', - ref: 'another-flow-or-cta', - extra: 'props', - } ); - } ); } ); } ); diff --git a/client/landing/stepper/declarative-flow/internals/index.tsx b/client/landing/stepper/declarative-flow/internals/index.tsx index 140751ef60fee..d94981c9fbd04 100644 --- a/client/landing/stepper/declarative-flow/internals/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/index.tsx @@ -196,7 +196,7 @@ export const FlowRenderer: React.FC< { flow: Flow } > = ( { flow } ) => { } }; - useSignUpStartTracking( { flow, currentStepRoute: currentStepRoute } ); + useSignUpStartTracking( { flow } ); return ( }>