wp-calypso icon indicating copy to clipboard operation
wp-calypso copied to clipboard

Track Migration Start for Reddit and Google Ads

Open briowill opened this issue 1 year ago • 12 comments

Related to # https://github.com/Automattic/martech/issues/3206

Migrations focused campaigns will be started and we are adding event tracking for Google and Reddit.

Proposed Changes

  1. Create a Tracking File
    • Add a new file to contain tracking codes for migration starts. This file can include any third-party trackers.
  2. Add Tracking Codes
    • Include Reddit and Google tracking codes in the newly created file.
  3. Update Migration Flow
    • Modify the migration flow to trigger a tracking event based on the user's referrer. Ensure that the appropriate tracking event fires for each applicable referrer.

Testing Instructions

  1. Apply patch
  2. Enable ad-tracking and cookie-banner in the development.json and spin up calypso locally.
  3. Open chrome console and navigate to the network tab
  4. Navigate to /setup/hosted-site-migration?ref=move-lp in the browser
  5. Once the page loads, you should see:
    • Reddit: A call to Search tracking event. See the screenshot below (filer by Reddit)
    • Google: A conversion event should be fired with tracking label 0qOACLyvvccZEP6YlcMD (Filter by label: 0qOACLyvvccZEP6YlcMD)

Reddit Screenshot 2024-07-26 at 3 15 30 PM

Google image

briowill avatar Jul 26 '24 20:07 briowill

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~37 bytes added 📈 [gzipped])

name                 parsed_size           gzip_size
entry-subscriptions        +71 B  (+0.0%)      +18 B  (+0.0%)
entry-stepper              +71 B  (+0.0%)      +18 B  (+0.0%)
entry-main                 +71 B  (+0.0%)      +18 B  (+0.0%)
entry-login                +71 B  (+0.0%)      +18 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~435 bytes added 📈 [gzipped])

name                        parsed_size           gzip_size
site-migration-flow              +506 B  (+0.9%)     +218 B  (+2.3%)
hosted-site-migration-flow       +506 B  (+0.9%)     +217 B  (+2.2%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

matticbot avatar Jul 26 '24 20:07 matticbot

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • blaze-dashboard
  • help-center
  • notifications
  • odyssey-stats

To test WordPress.com changes, run install-plugin.sh $pluginSlug update/add-migration-start-tracking on your sandbox.

matticbot avatar Aug 02 '24 22:08 matticbot

@gmovr Could you give this another review, I deferred the ad tracking in the recordMigrationStart()

briowill avatar Aug 07 '24 23:08 briowill

From an ad-tracking perspective, this works. I tested that the trackers don't fire unless consent is given, and I can see the Google event in the browser's developer tools.

I'm not seeing the Reddit event, however. I'm not sure why, because if I fire off window.rdt && window.rdt( 'track', 'Search' ); in the console, I see the network request working fine.

It's not related to the Search event, because this also does not work:

	window.rdt('track', 'Custom', {
		customEventName:'Test Event',
	});

But if I add a debugger; statement right before the window.rdt call, I can check in the console that window.rdt is defined. If I remove the useEffect hook, it does fire, but then it fires twice.

FYI: I also added some folks more familiar with Stepper here.

gmovr avatar Aug 09 '24 11:08 gmovr

The Search event in a setTimeout will fire, but it doesn't have the right ID (it's undefined), which is probably because it will fire before the pixel is initialized.

These changes will make Reddit fire, but it will fire twice. But now it has the correct ID, because it fires at the correct time (I think).

diff --git a/client/landing/stepper/declarative-flow/site-migration-flow.ts b/client/landing/stepper/declarative-flow/site-migration-flow.ts
index 14d671019c..eb8d282964 100644
--- a/client/landing/stepper/declarative-flow/site-migration-flow.ts
+++ b/client/landing/stepper/declarative-flow/site-migration-flow.ts
@@ -57,6 +57,9 @@ const siteMigration: Flow = {
 		return stepsWithRequiredLogin( [ ...baseSteps, ...hostedVariantSteps ] );
 	},
 
+	useSideEffect() {
+		recordMigrationStart();
+	},
 	useAssertConditions(): AssertConditionResult {
 		const { siteSlug, siteId } = useSiteData();
 		const { isAdmin } = useIsSiteAdmin();
@@ -126,10 +129,6 @@ const siteMigration: Flow = {
 			window.location.assign( to );
 		};
 
-		useEffect( () => {
-			recordMigrationStart();
-		}, [ urlQueryParams ] );
-
 		// Call triggerGuidesForStep for the current step
 		useEffect( () => {
 			triggerGuidesForStep( flowName, currentStep, siteId );
diff --git a/client/lib/analytics/ad-tracking/ad-track-migration-start.ts b/client/lib/analytics/ad-tracking/ad-track-migration-start.ts
index 78d64238ef..2efbf5e72f 100644
--- a/client/lib/analytics/ad-tracking/ad-track-migration-start.ts
+++ b/client/lib/analytics/ad-tracking/ad-track-migration-start.ts

@@ -25,8 +25,6 @@ const adTrackGoogleEvent = (): void => {
 };
 
 export const recordMigrationStart = (): void => {
-	setTimeout( () => {
-		adTrackRedditEvent();
-		adTrackGoogleEvent();
-	}, 0 );
+	adTrackRedditEvent();
+	adTrackGoogleEvent();
 };

gmovr avatar Aug 09 '24 12:08 gmovr

Doing this will make the code run exactly once 🤷

useSideEffect( currentStep) {
	useEffect( () => { debugger },[ currentStep ] );
},

But if you add the Reddit call there, it will run, but the network request doesn't go out. I'm sure I'm doing something wrong, though :)

useSideEffect( currentStep) {
	useEffect( () => {
			recordMigrationStart()
		} ,[ currentStep ] );
},

If you add a debugger inside the reddit event, you can see that window.rdt.callQueue has the event queued up. 🤔

gmovr avatar Aug 09 '24 12:08 gmovr

Just digging a bit more. This will solve the event not firing, but the ID isn't defined. I wonder if we need to sort this between the setup.js and load-tracking-scripts.js

diff --git a/client/lib/analytics/ad-tracking/setup.js b/client/lib/analytics/ad-tracking/setup.js
index fa18e1d39a..3f4e618c9a 100644
--- a/client/lib/analytics/ad-tracking/setup.js
+++ b/client/lib/analytics/ad-tracking/setup.js
@@ -226,7 +226,7 @@ function setupRedditGlobal() {
                        window.rdt.sendEvent ? window.rdt.sendEvent( ...args ) : window.rdt.callQueue.push( args );
                };

-       window.rdt.callQueue = [];
+       window.rdt.callQueue = window.rdt.callQueue || [];
 }

 function setupGtag() {

These are adapted from the Reddit pixel from the platform:

<!-- Reddit Pixel -->
<script>
!function(w,d){if(!w.rdt){var p=w.rdt=function(){p.sendEvent?p.sendEvent.apply(p,arguments):p.callQueue.push(arguments)};p.callQueue=[];var t=d.createElement("script");t.src="https://www.redditstatic.com/ads/pixel.js",t.async=!0;var s=d.getElementsByTagName("script")[0];s.parentNode.insertBefore(t,s)}}(window,document);rdt('init','a2_ehx23cq176s3');rdt('track', 'PageVisit');
</script>
<!-- DO NOT MODIFY UNLESS TO REPLACE A USER IDENTIFIER -->
<!-- End Reddit Pixel -->

gmovr avatar Aug 09 '24 13:08 gmovr

This seems to fix the bug, so it's a step in the right direction. I guess on most pages, the order of operations is correct in most cases, but maybe not when called in the Stepper flows.

diff --git a/client/lib/analytics/ad-tracking/load-tracking-scripts.js b/client/lib/analytics/ad-tracking/load-tracking-scripts.js
index 170d41fa01..ee471e6094 100644
--- a/client/lib/analytics/ad-tracking/load-tracking-scripts.js
+++ b/client/lib/analytics/ad-tracking/load-tracking-scripts.js
@@ -162,15 +162,6 @@ function initLoadedTrackingScripts() {
 		window.pintrk( 'load', TRACKING_IDS.pinterestInit, params );
 	}
 
-	if ( mayWeTrackByTracker( 'reddit' ) ) {
-		const params = {
-			optOut: false,
-			useDecimalCurrencyValues: true,
-		};
-
-		window.rdt( 'init', WPCOM_REDDIT_PIXEL_ID, params );
-	}
-
 	debug( 'loadTrackingScripts: init done' );
 }
 
diff --git a/client/lib/analytics/ad-tracking/setup.js b/client/lib/analytics/ad-tracking/setup.js
index fa18e1d39a..dcb047bd6f 100644
--- a/client/lib/analytics/ad-tracking/setup.js
+++ b/client/lib/analytics/ad-tracking/setup.js
@@ -8,6 +8,7 @@ import {
 	ADROLL_PURCHASE_PIXEL_URL_1,
 	ADROLL_PURCHASE_PIXEL_URL_2,
 	TRACKING_IDS,
+	WPCOM_REDDIT_PIXEL_ID
 } from './constants';
 
 export function setup() {
@@ -223,10 +224,14 @@ function setupRedditGlobal() {
 	window.rdt =
 		window.rdt ||
 		function ( ...args ) {
+			window.rdt.callQueue = window.rdt.callQueue || [];
 			window.rdt.sendEvent ? window.rdt.sendEvent( ...args ) : window.rdt.callQueue.push( args );
 		};
-
-	window.rdt.callQueue = [];
+		const params = {
+			optOut: false,
+			useDecimalCurrencyValues: true,
+		};
+		window.rdt( 'init', WPCOM_REDDIT_PIXEL_ID, params );
 }
 
 function setupGtag() {

gmovr avatar Aug 09 '24 13:08 gmovr

@gmovr what is the problem exactly?

I'm neither confident (nor familiar) with order of operations in Stepper tbh. Everything's a hook, attached to some object, I don't see how one can build anything without a solid understanding of what goes where. Then again, I think folks are convinced that's 100% not the case, so I'm at best wrong.

Anything to do with the setTimeout call though? Perhaps clearing the timeout in exit closure?

chriskmnds avatar Aug 09 '24 15:08 chriskmnds

Just digging a bit more. This will solve the event not firing, but the ID isn't defined. I wonder if we need to sort this between the setup.js and load-tracking-scripts.js

Ran a few tests in load-tracking-scripts.js and the loadTrackingScripts function throws and error for twitter script load

https://github.com/Automattic/wp-calypso/blob/05023d1b7c1eda993d76e0d323ce109ecefe20c5/client/lib/analytics/ad-tracking/load-tracking-scripts.js#L25

Once an error is thrown initLoadedTrackingScripts() is prevented from loading. The reddit script wasn't being initialized. This is apart of the issue but not all.

https://github.com/Automattic/wp-calypso/blob/05023d1b7c1eda993d76e0d323ce109ecefe20c5/client/lib/analytics/ad-tracking/load-tracking-scripts.js#L43-L49

briowill avatar Aug 10 '24 02:08 briowill

@gmovr what is the problem exactly?

I'm neither confident (nor familiar) with order of operations in Stepper tbh. Everything's a hook, attached to some object, I don't see how one can build anything without a solid understanding of what goes where. Then again, I think folks are convinced that's 100% not the case, so I'm at best wrong.

Anything to do with the setTimeout call though? Perhaps clearing the timeout in exit closure?

I think it's sorted now. In my testing, it was the Reddit pixel being fired off before it was initialized, causing it to be sent with no pixel ID.

The latest change from Brian, using the side effect method in Stepper, seems to have fixed it.

So, no problem at the moment. I added you as a reviewer as someone who's worked a bit more with Stepper, just based on the recent history of the file, in case we're missing anything obvious.

gmovr avatar Aug 12 '24 07:08 gmovr

This PR has been marked as stale. This happened because:

  • It has been inactive for the past 3 months.
  • It hasn't been labeled `[Pri] BLOCKER`, `[Pri] High`, `[Status] Keep Open`, etc.

If this PR is still useful, please do a trunk merge or rebase and otherwise make sure it's up to date and has clear testing instructions. You may also want to ping possible reviewers in case they've forgotten about it. Please close this PR if you think it's not valid anymore — if you do, please add a brief explanation.

If the PR is not updated (or at least commented on) in another month, it will be automatically closed.

github-actions[bot] avatar Jul 30 '25 18:07 github-actions[bot]