From a6cf204f06e9ba7bed1e567ba4d0ec997efa5860 Mon Sep 17 00:00:00 2001 From: Christos Date: Mon, 30 Sep 2024 14:25:36 +0300 Subject: [PATCH] Stepper: Ensure clean timers on `calypso_signup_start` (#94841) --- client/landing/stepper/constants.ts | 2 + .../analytics/record-signup-start.ts | 20 +++++++ .../use-sign-up-start-tracking/index.tsx | 26 ++++++++-- .../use-sign-up-start-tracking/test/index.tsx | 52 +++++++++++++++---- 4 files changed, 87 insertions(+), 13 deletions(-) create mode 100644 client/landing/stepper/declarative-flow/internals/analytics/record-signup-start.ts diff --git a/client/landing/stepper/constants.ts b/client/landing/stepper/constants.ts index afbe9bc4de2285..8a732d4eb39c82 100644 --- a/client/landing/stepper/constants.ts +++ b/client/landing/stepper/constants.ts @@ -26,6 +26,7 @@ export const STEPPER_TRACKS_EVENT_STEP_NAV_GO_NEXT = 'calypso_signup_step_nav_ne export const STEPPER_TRACKS_EVENT_STEP_NAV_GO_TO = 'calypso_signup_step_nav_go_to'; export const STEPPER_TRACKS_EVENT_STEP_NAV_EXIT_FLOW = 'calypso_signup_step_nav_exit_flow'; export const STEPPER_TRACKS_EVENT_STEP_COMPLETE = 'calypso_signup_actions_complete_step'; +export const STEPPER_TRACKS_EVENT_SIGNUP_START = 'calypso_signup_start'; export const STEPPER_TRACKS_EVENTS_STEP_NAV = < const >[ STEPPER_TRACKS_EVENT_STEP_NAV_SUBMIT, @@ -38,4 +39,5 @@ export const STEPPER_TRACKS_EVENTS_STEP_NAV = < const >[ export const STEPPER_TRACKS_EVENTS = < const >[ ...STEPPER_TRACKS_EVENTS_STEP_NAV, STEPPER_TRACKS_EVENT_STEP_COMPLETE, + STEPPER_TRACKS_EVENT_SIGNUP_START, ]; diff --git a/client/landing/stepper/declarative-flow/internals/analytics/record-signup-start.ts b/client/landing/stepper/declarative-flow/internals/analytics/record-signup-start.ts new file mode 100644 index 00000000000000..8e348eac4cfde4 --- /dev/null +++ b/client/landing/stepper/declarative-flow/internals/analytics/record-signup-start.ts @@ -0,0 +1,20 @@ +import { recordTracksEvent } from '@automattic/calypso-analytics'; +import { resolveDeviceTypeByViewPort } from '@automattic/viewport'; +import { STEPPER_TRACKS_EVENT_SIGNUP_START } from 'calypso/landing/stepper/constants'; + +export interface RecordSignupStartProps { + flow: string; + ref: string; + optionalProps?: Record< string, string | number | null >; +} + +const recordSignupStart = ( { flow, ref, optionalProps }: RecordSignupStartProps ) => { + recordTracksEvent( STEPPER_TRACKS_EVENT_SIGNUP_START, { + flow, + ref, + device: resolveDeviceTypeByViewPort(), + ...optionalProps, + } ); +}; + +export default recordSignupStart; 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 2318ada9766c59..1a7c7a3411167e 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,6 +1,9 @@ import { useEffect, useMemo } from 'react'; import { useSearchParams } from 'react-router-dom'; -import { recordSignupStart } from 'calypso/lib/analytics/signup'; +import recordSignupStart from 'calypso/landing/stepper/declarative-flow/internals/analytics/record-signup-start'; +import { adTrackSignupStart } from 'calypso/lib/analytics/ad-tracking'; +import { gaRecordEvent } from 'calypso/lib/analytics/ga'; +import { setSignupStartTime } from 'calypso/signup/storageUtils'; import { type Flow } from '../../types'; /** @@ -17,7 +20,6 @@ export const useSignUpStartTracking = ( { flow }: Props ) => { const flowName = flow.name; const isSignupFlow = flow.isSignupFlow; const signupStartEventProps = flow.useSignupStartEventProps?.(); - const extraProps = useMemo( () => ( { ...signupStartEventProps, @@ -26,11 +28,27 @@ export const useSignUpStartTracking = ( { flow }: Props ) => { [ signupStartEventProps, flowVariant ] ); + /** + * Timers and other analytics + * + * Important: Ideally, this hook should only run once per signup (`isSignupFlow`) session. + * Avoid introducing more dependencies. + */ useEffect( () => { if ( ! isSignupFlow ) { return; } - recordSignupStart( flowName, ref, extraProps || {} ); - }, [ extraProps, flowName, ref, isSignupFlow ] ); + setSignupStartTime(); + // Google Analytics + gaRecordEvent( 'Signup', 'calypso_signup_start' ); + // Marketing + adTrackSignupStart( flowName ); + }, [ isSignupFlow, flowName ] ); + + if ( ! isSignupFlow ) { + return; + } + + recordSignupStart( { flow: flowName, ref, optionalProps: extraProps || {} } ); }; 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 5cf1282150d231..1a48fa38f8285a 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 @@ -1,11 +1,13 @@ /** * @jest-environment jsdom */ - -import { recordTracksEvent } from '@automattic/calypso-analytics'; import { addQueryArgs } from '@wordpress/url'; import React from 'react'; import { MemoryRouter } from 'react-router-dom'; +import recordSignupStart from 'calypso/landing/stepper/declarative-flow/internals/analytics/record-signup-start'; +import { adTrackSignupStart } from 'calypso/lib/analytics/ad-tracking'; +import { gaRecordEvent } from 'calypso/lib/analytics/ga'; +import { setSignupStartTime } from 'calypso/signup/storageUtils'; import { renderHookWithProvider } from 'calypso/test-helpers/testing-library'; import { useSignUpStartTracking } from '../'; import type { Flow, StepperStep } from '../../../types'; @@ -28,6 +30,10 @@ const signUpFlow: Flow = { }; jest.mock( '@automattic/calypso-analytics' ); +jest.mock( 'calypso/landing/stepper/declarative-flow/internals/analytics/record-signup-start' ); +jest.mock( 'calypso/lib/analytics/ad-tracking' ); +jest.mock( 'calypso/lib/analytics/ga' ); +jest.mock( 'calypso/signup/storageUtils' ); const render = ( { flow, queryParams = {} } ) => { return renderHookWithProvider( @@ -53,13 +59,13 @@ describe( 'useSignUpTracking', () => { it( 'does not track event when the flow is not a isSignupFlow', () => { render( { flow: regularFlow } ); - expect( recordTracksEvent ).not.toHaveBeenCalled(); + expect( recordSignupStart ).not.toHaveBeenCalled(); } ); it( 'does not track event when the flow is not a isSignupFlow and the signup flag is set', () => { render( { flow: regularFlow, queryParams: { start: 1 } } ); - expect( recordTracksEvent ).not.toHaveBeenCalled(); + expect( recordSignupStart ).not.toHaveBeenCalled(); } ); describe( 'sign-up-flow', () => { @@ -69,9 +75,10 @@ describe( 'useSignUpTracking', () => { queryParams: { ref: 'another-flow-or-cta' }, } ); - expect( recordTracksEvent ).toHaveBeenCalledWith( 'calypso_signup_start', { + expect( recordSignupStart ).toHaveBeenCalledWith( { flow: 'sign-up-flow', ref: 'another-flow-or-cta', + optionalProps: {}, } ); } ); @@ -84,10 +91,12 @@ describe( 'useSignUpTracking', () => { queryParams: { ref: 'another-flow-or-cta' }, } ); - expect( recordTracksEvent ).toHaveBeenCalledWith( 'calypso_signup_start', { + expect( recordSignupStart ).toHaveBeenCalledWith( { flow: 'sign-up-flow', ref: 'another-flow-or-cta', - extra: 'props', + optionalProps: { + extra: 'props', + }, } ); } ); @@ -99,11 +108,36 @@ describe( 'useSignUpTracking', () => { } satisfies Flow, } ); - expect( recordTracksEvent ).toHaveBeenCalledWith( 'calypso_signup_start', { + expect( recordSignupStart ).toHaveBeenCalledWith( { flow: 'sign-up-flow', - flow_variant: 'variant-slug', + optionalProps: { + flow_variant: 'variant-slug', + }, ref: '', } ); } ); + + it( 'sets the signup-start timer only on initial mount (assuming static flowName and isSignupFlow)', () => { + const { rerender } = render( { + flow: { + ...signUpFlow, + } satisfies Flow, + } ); + + rerender(); + expect( setSignupStartTime ).toHaveBeenCalledTimes( 1 ); + } ); + + it( 'records Google-Analytics and AD tracking only on initial mount (assuming static flowName and isSignupFlow)', () => { + const { rerender } = render( { + flow: { + ...signUpFlow, + } satisfies Flow, + } ); + + rerender(); + expect( gaRecordEvent ).toHaveBeenCalledTimes( 1 ); + expect( adTrackSignupStart ).toHaveBeenCalledTimes( 1 ); + } ); } ); } );