Skip to content

[11722] Add GA event tracking for the Analytics setup screen. #11776

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Nov 25, 2025
Merged

[11722] Add GA event tracking for the Analytics setup screen. #11776

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f8b25f2
Add GA event tracking for the Analytics setup screen.
ankitrox Nov 17, 2025
6b6aa0c
Remove redundant code.
ankitrox Nov 17, 2025
de48db5
Fix: reference error.
ankitrox Nov 17, 2025
c18d236
Merge branch 'develop' into enhancement/11722-ga-event-analytics-setup.
ankitrox Nov 18, 2025
5daa659
Optimize the conditional statement.
ankitrox Nov 18, 2025
00d55fe
Merge branch 'develop' into enhancement/11722-ga-event-analytics-setup.
ankitrox Nov 20, 2025
54cd414
Address CR feedback.
ankitrox Nov 20, 2025
0c0865f
Merge branch 'develop' into enhancement/11722-ga-event-analytics-setup.
ankitrox Nov 24, 2025
66a9748
Address MR feedback.
ankitrox Nov 24, 2025
8867812
Fix: JS test.
ankitrox Nov 24, 2025
8d9b9ee
Add afterEach.
ankitrox Nov 25, 2025
5087bf3
Update snapshot.
ankitrox Nov 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions assets/js/components/setup/ModuleSetup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { deleteItem } from '@/js/googlesitekit/api/cache';
import { trackEvent } from '@/js/util';
import { useFeature } from '@/js/hooks/useFeature';
import useQueryArg from '@/js/hooks/useQueryArg';
import useViewContext from '@/js/hooks/useViewContext';
import HelpMenu from '@/js/components/help/HelpMenu';
import { Cell, Grid, Row } from '@/js/material-components';
import Header from '@/js/components/Header';
Expand All @@ -49,6 +50,7 @@ import ExitSetup from '@/js/components/setup/ExitSetup';
import ProgressIndicator from '@/js/components/ProgressIndicator';

export default function ModuleSetup( { moduleSlug } ) {
const viewContext = useViewContext();
const { navigateTo } = useDispatch( CORE_LOCATION );
const setupFlowRefreshEnabled = useFeature( 'setupFlowRefresh' );
const [ showProgress ] = useQueryArg( 'showProgress' );
Expand All @@ -71,11 +73,13 @@ export default function ModuleSetup( { moduleSlug } ) {
async ( redirectURL ) => {
await deleteItem( 'module_setup' );

await trackEvent(
'moduleSetup',
'complete_module_setup',
moduleSlug
);
const completionEventArgs = isInitialSetupFlow
Copy link
Collaborator

@abdelmalekkkkk abdelmalekkkkk Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use a simple if/else here? It would be more readable in my opinion.

Copy link
Collaborator Author

@ankitrox ankitrox Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion @abdelmalekkkkk.

I think we should stick to ternary operator here because of following reasons:

  1. This is fairly simple expression.
  2. If we use if...else statement we may have to either call trackEvent twice or need to use let completionEventArgs which is susceptible to change which is a source of potential side-effect bugs.

Copy link
Collaborator

@techanvil techanvil Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ankitrox, thanks for explaining your rationale here, however, I would agree with @abdelmalekkkkk that it would be more readable to use an if/else. It won't result in calling trackEvent() twice, simply writing it twice, which seems fine when considering the improved legibility:

			if ( isInitialSetupFlow ) {
				await trackEvent(
					`${ viewContext }_setup`,
					'setup_flow_v3_complete_analytics_step'
				);
			} else {
				await trackEvent(
					'moduleSetup',
					'complete_module_setup',
					moduleSlug
				);
			}

? [
`${ viewContext }_setup`,
'setup_flow_v3_complete_analytics_step',
]
: [ 'moduleSetup', 'complete_module_setup', moduleSlug ];
await trackEvent( ...completionEventArgs );

if ( redirectURL ) {
navigateTo( redirectURL );
Expand Down Expand Up @@ -106,7 +110,21 @@ export default function ModuleSetup( { moduleSlug } ) {
await trackEvent( 'moduleSetup', 'cancel_module_setup', moduleSlug );
}, [ moduleSlug ] );

const isInitialSetupFlow =
Copy link
Collaborator

@abdelmalekkkkk abdelmalekkkkk Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this to the top of the component? This works perfectly fine but would sit nicely below the showProgress and the other declarations.

setupFlowRefreshEnabled &&
moduleSlug === MODULE_SLUG_ANALYTICS_4 &&
showProgress === 'true';

useMount( () => {
if ( isInitialSetupFlow ) {
trackEvent(
`${ viewContext }_setup`,
'setup_flow_v3_view_analytics_step'
);

return;
}

trackEvent( 'moduleSetup', 'view_module_setup', moduleSlug );
} );

Expand All @@ -116,11 +134,6 @@ export default function ModuleSetup( { moduleSlug } ) {

const { SetupComponent } = module;

const isInitialSetupFlow =
setupFlowRefreshEnabled &&
moduleSlug === MODULE_SLUG_ANALYTICS_4 &&
showProgress === 'true';

return (
<Fragment>
<Header
Expand Down
33 changes: 33 additions & 0 deletions assets/js/components/setup/ModuleSetup.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,18 @@ import { VIEW_CONTEXT_MODULE_SETUP } from '@/js/googlesitekit/constants';
import { mockLocation } from '../../../../tests/js/mock-browser-utils';
import { CORE_USER } from '@/js/googlesitekit/datastore/user/constants';
import { CORE_MODULES } from '@/js/googlesitekit/modules/datastore/constants';
import * as tracking from '@/js/util/tracking';

const mockTrackEvent = jest.spyOn( tracking, 'trackEvent' );
mockTrackEvent.mockImplementation( () => Promise.resolve() );

describe( 'ModuleSetup', () => {
mockLocation();

let registry;

beforeEach( () => {
mockTrackEvent.mockClear();
Copy link
Collaborator

@techanvil techanvil Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per my other comment, please move this line to an afterEach() callback.

registry = createTestRegistry();
registry.dispatch( CORE_USER ).receiveGetDismissedItems( [] );
registry.dispatch( CORE_USER ).receiveGetDismissedPrompts( {} );
Expand Down Expand Up @@ -175,6 +180,20 @@ describe( 'ModuleSetup', () => {
it( 'should match the snapshot', () => {
expect( container ).toMatchSnapshot();
} );

it( 'tracks only the initial setup analytics view event', () => {
expect( mockTrackEvent ).toHaveBeenCalledWith(
`${ VIEW_CONTEXT_MODULE_SETUP }_setup`,
'setup_flow_v3_view_analytics_step'
);
const genericViewCall = mockTrackEvent.mock.calls.find(
( call ) =>
call[ 0 ] === 'moduleSetup' &&
call[ 1 ] === 'view_module_setup' &&
call[ 2 ] === MODULE_SLUG_ANALYTICS_4
);
expect( genericViewCall ).toBeUndefined();
} );
} );

describe( 'not initial setup flow', () => {
Expand Down Expand Up @@ -230,6 +249,20 @@ describe( 'ModuleSetup', () => {
it( 'should match the snapshot', () => {
expect( container ).toMatchSnapshot();
} );

it( 'tracks only the generic module setup view event', () => {
expect( mockTrackEvent ).toHaveBeenCalledWith(
'moduleSetup',
'view_module_setup',
MODULE_SLUG_ANALYTICS_4
);
const initialAnalyticsCall = mockTrackEvent.mock.calls.find(
( call ) =>
call[ 0 ] === `${ VIEW_CONTEXT_MODULE_SETUP }_setup` &&
call[ 1 ] === 'setup_flow_v3_view_analytics_step'
);
expect( initialAnalyticsCall ).toBeUndefined();
} );
} );
} );

Expand Down
10 changes: 5 additions & 5 deletions assets/js/components/setup/__snapshots__/ModuleSetup.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ exports[`ModuleSetup Analytics 4 not initial setup flow should match the snapsho
>
<label
class="mdc-floating-label mdc-floating-label--float-above"
for="googlesitekit-select-33"
for="googlesitekit-select-36"
style=""
>
Account
Expand Down Expand Up @@ -663,7 +663,7 @@ exports[`ModuleSetup Analytics 4 not initial setup flow should match the snapsho
>
<label
class="mdc-floating-label mdc-floating-label--float-above"
for="googlesitekit-select-34"
for="googlesitekit-select-37"
style=""
>
Property
Expand Down Expand Up @@ -705,7 +705,7 @@ exports[`ModuleSetup Analytics 4 not initial setup flow should match the snapsho
>
<label
class="mdc-floating-label mdc-floating-label--float-above"
for="googlesitekit-select-35"
for="googlesitekit-select-38"
style=""
>
Web Data Stream
Expand Down Expand Up @@ -741,7 +741,7 @@ exports[`ModuleSetup Analytics 4 not initial setup flow should match the snapsho
<input
checked=""
class="mdc-switch__native-control"
id="googlesitekit-switch-11"
id="googlesitekit-switch-12"
readonly=""
role="switch"
type="checkbox"
Expand All @@ -751,7 +751,7 @@ exports[`ModuleSetup Analytics 4 not initial setup flow should match the snapsho
</div>
<label
class=""
for="googlesitekit-switch-11"
for="googlesitekit-switch-12"
>
Enable enhanced measurement
</label>
Expand Down
19 changes: 11 additions & 8 deletions assets/js/modules/analytics-4/components/common/AccountCreate/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ export default function AccountCreate( { className } ) {
}, [ hasAccountCreateForm, siteName, siteURL, timezone, setValues ] );

const showProgress = getQueryArg( location.href, 'showProgress' );
const isInitialSetupFlow = !! showProgress && setupFlowRefreshEnabled;

const handleSubmit = useCallback( async () => {
const scopes = [];
Expand Down Expand Up @@ -192,11 +193,14 @@ export default function AccountCreate( { className } ) {
}

setValues( FORM_ACCOUNT_CREATE, { autoSubmit: false } );
await trackEvent(
`${ viewContext }_analytics`,
'create_account',
'proxy'
);

const createAccountEventArgs = isInitialSetupFlow
Copy link
Collaborator

@abdelmalekkkkk abdelmalekkkkk Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Copy link
Collaborator Author

@ankitrox ankitrox Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Copy link
Collaborator

@techanvil techanvil Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reply above also applies here, please do update this to use an if/else.

? [
`${ viewContext }_setup`,
'setup_flow_v3_create_analytics_account',
]
: [ `${ viewContext }_analytics`, 'create_account', 'proxy' ];
await trackEvent( ...createAccountEventArgs );

const { error } = await createAccount( {
showProgress: showProgress === 'true',
Expand All @@ -210,10 +214,11 @@ export default function AccountCreate( { className } ) {
hasEditScope,
hasGTMScope,
setValues,
viewContext,
isInitialSetupFlow,
Copy link
Collaborator

@abdelmalekkkkk abdelmalekkkkk Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why you added isInitialSetupFlow and viewContext to the dependency array here but not in ModuleSetup.js? Doesn't seem like a big deal, but I'm wondering if we can keep things consistent.

Copy link
Collaborator Author

@ankitrox ankitrox Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good catch @abdelmalekkkkk , thanks for spotting it.

I've added it now.

createAccount,
showProgress,
setPermissionScopeError,
viewContext,
setConversionTrackingEnabled,
saveConversionTrackingSettings,
] );
Expand Down Expand Up @@ -242,8 +247,6 @@ export default function AccountCreate( { className } ) {
return <ProgressBar />;
}

const isInitialSetupFlow = !! showProgress && setupFlowRefreshEnabled;

return (
<div className={ className }>
<StoreErrorNotices
Expand Down
78 changes: 78 additions & 0 deletions assets/js/modules/analytics-4/components/common/AccountCreate/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
* Internal dependencies
*/
import {
cleanup,
createTestRegistry,
fireEvent,
muteFetch,
Expand All @@ -42,9 +43,14 @@ import { MODULE_SLUG_ANALYTICS_4 } from '@/js/modules/analytics-4/constants';
import { createCacheKey } from '@/js/googlesitekit/api';
import { getKeys, setItem } from '@/js/googlesitekit/api/cache';
import AccountCreate from '.';
import * as tracking from '@/js/util/tracking';
import { VIEW_CONTEXT_MODULE_SETUP } from '@/js/googlesitekit/constants';
import { CORE_SITE } from '@/js/googlesitekit/datastore/site/constants';
import { MODULE_SLUG_SEARCH_CONSOLE } from '@/js/modules/search-console/constants';

const mockTrackEvent = jest.spyOn( tracking, 'trackEvent' );
mockTrackEvent.mockImplementation( () => Promise.resolve() );

const REGEX_REST_CONVERSION_TRACKING_SETTINGS = new RegExp(
'^/google-site-kit/v1/core/site/data/conversion-tracking'
);
Expand Down Expand Up @@ -312,5 +318,77 @@ describe( 'AccountCreate', () => {
REGEX_REST_CONVERSION_TRACKING_SETTINGS
);
} );

describe( 'event tracking', () => {
beforeEach( () => {
mockTrackEvent.mockClear();
cleanup();
} );

it( 'should track initial setup flow create account event when showProgress=true', async () => {
Copy link
Collaborator

@abdelmalekkkkk abdelmalekkkkk Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the label needs to be something along the lines of "during initial setup flow" instead of "when showProgress=true", because showProgress is not the only condition.

global.location.href =
'http://example.com/wp-admin/admin.php?page=googlesitekit-dashboard&slug=analytics-4&reAuth=true&showProgress=true';

( { getByRole, waitForRegistry, rerender } = render(
<AccountCreate />,
{
registry,
viewContext: VIEW_CONTEXT_MODULE_SETUP,
features: [ 'setupFlowRefresh' ],
}
) );

fireEvent.click(
getByRole( 'button', { name: 'Create Account' } )
);

await waitForRegistry();

expect( mockTrackEvent ).toHaveBeenCalledWith(
`${ VIEW_CONTEXT_MODULE_SETUP }_setup`,
'setup_flow_v3_create_analytics_account'
);

const genericCreateAccountCall = mockTrackEvent.mock.calls.find(
( call ) =>
call[ 0 ]?.endsWith( '_analytics' ) &&
call[ 1 ] === 'create_account'
);
expect( genericCreateAccountCall ).toBeUndefined();
} );

it( 'should track generic create account event when showProgress is not true', async () => {
Copy link
Collaborator

@abdelmalekkkkk abdelmalekkkkk Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

global.location.href =
'http://example.com/wp-admin/admin.php?page=googlesitekit-dashboard&slug=analytics-4&reAuth=true';
( { getByRole, waitForRegistry, rerender } = render(
<AccountCreate />,
{
registry,
viewContext: VIEW_CONTEXT_MODULE_SETUP,
}
) );

fireEvent.click(
getByRole( 'button', { name: 'Create Account' } )
);

await waitForRegistry();

const genericCreateAccountCall = mockTrackEvent.mock.calls.find(
( call ) =>
call[ 0 ]?.endsWith( '_analytics' ) &&
call[ 1 ] === 'create_account' &&
call[ 2 ] === 'proxy'
);
expect( genericCreateAccountCall ).toBeDefined();

const initialSetupCall = mockTrackEvent.mock.calls.find(
( call ) =>
call[ 0 ] === `${ VIEW_CONTEXT_MODULE_SETUP }_setup` &&
call[ 1 ] === 'setup_flow_v3_create_analytics_account'
);
expect( initialSetupCall ).toBeUndefined();
} );
} );
} );
} );
Loading