Merge pull request #2330 from google/fix/1999-track-event-value

Fix trackEvent with value
google · Nov 5, 2020 · 40ff977 · 40ff977
2 parents cbae2ae + c9521f5
commit 40ff977
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 21 deletions.
diff --git a/assets/js/components/data/index.test.js b/assets/js/components/data/index.test.js
@@ -66,7 +66,7 @@ describe( 'dataAPI', () => {
 				expect( eventName ).toEqual( 'GET:test-type/test-identifier/data/test-datapoint' );
 				expect( eventData.event_category ).toEqual( 'api_error' );
 				expect( eventData.event_label ).toEqual( 'Internal server error (code: internal_server_error)' );
-				expect( eventData.event_value ).toEqual( 500 );
+				expect( eventData.value ).toEqual( 500 );
 			}
 		} );
 	} );
@@ -90,7 +90,7 @@ describe( 'dataAPI', () => {
 				expect( eventName ).toEqual( 'POST:test-type/test-identifier/data/test-datapoint' );
 				expect( eventData.event_category ).toEqual( 'api_error' );
 				expect( eventData.event_label ).toEqual( 'Internal server error (code: internal_server_error)' );
-				expect( eventData.event_value ).toEqual( 500 );
+				expect( eventData.value ).toEqual( 500 );
 			}
 		} );
 	} );
@@ -166,7 +166,7 @@ describe( 'dataAPI', () => {
 			expect( eventName ).toEqual( 'POST:test-type/test-identifier/data/test-datapoint' );
 			expect( eventData.event_category ).toEqual( 'api_error' );
 			expect( eventData.event_label ).toEqual( 'Internal server error (code: internal_server_error, reason: internal_server_error)' );
-			expect( eventData.event_value ).toEqual( 500 );
+			expect( eventData.value ).toEqual( 500 );
 		} );
 
 		it( 'should call trackEvent for each error in combinedGet with multiple errors', async () => {
@@ -207,13 +207,13 @@ describe( 'dataAPI', () => {
 			expect( eventName ).toEqual( 'POST:test-type/test-identifier/data/test-datapoint' );
 			expect( eventData.event_category ).toEqual( 'api_error' );
 			expect( eventData.event_label ).toEqual( 'Internal server error (code: internal_server_error, reason: internal_server_error)' );
-			expect( eventData.event_value ).toEqual( 500 );
+			expect( eventData.value ).toEqual( 500 );
 			[ event, eventName, eventData ] = dataLayerPushSpy.mock.calls[ 1 ][ 0 ];
 			expect( event ).toEqual( 'event' );
 			expect( eventName ).toEqual( 'POST:test-type/analytics/data/test-datapoint-3' );
 			expect( eventData.event_category ).toEqual( 'api_error' );
 			expect( eventData.event_label ).toEqual( 'Unknown error (code: unknown_error, reason: unknown_error)' );
-			expect( eventData.event_value ).toEqual( 503 );
+			expect( eventData.value ).toEqual( 503 );
 		} );
 	} );
 } );
diff --git a/assets/js/googlesitekit/api/index.test.js b/assets/js/googlesitekit/api/index.test.js
@@ -308,7 +308,7 @@ describe( 'googlesitekit.api', () => {
 				expect( eventName ).toEqual( 'GET:test-type/test-identifier/data/test-datapoint' );
 				expect( eventData.event_category ).toEqual( 'api_error' );
 				expect( eventData.event_label ).toEqual( 'Internal server error (code: internal_server_error)' );
-				expect( eventData.event_value ).toEqual( 500 );
+				expect( eventData.value ).toEqual( 500 );
 			}
 		} );
 	} );
@@ -491,7 +491,7 @@ describe( 'googlesitekit.api', () => {
 				expect( eventName ).toEqual( 'POST:test-type/test-identifier/data/test-datapoint' );
 				expect( eventData.event_category ).toEqual( 'api_error' );
 				expect( eventData.event_label ).toEqual( 'Internal server error (code: internal_server_error)' );
-				expect( eventData.event_value ).toEqual( 500 );
+				expect( eventData.value ).toEqual( 500 );
 			}
 		} );
 	} );

diff --git a/assets/js/util/api.test.js b/assets/js/util/api.test.js
@@ -52,7 +52,7 @@ describe( 'trackAPIError', () => {
 		expect( eventName ).toEqual( 'test-method:test-type/test-identifier/data/test-datapoint' );
 		expect( eventData.event_category ).toEqual( 'api_error' );
 		expect( eventData.event_label ).toEqual( 'test-error-message (code: test-error-code, reason: test-error-reason)' );
-		expect( eventData.event_value ).toEqual( 'test-error-code' );
+		expect( eventData.value ).toEqual( 'test-error-code' );
 	} );
 
 	it( 'tracks API error message & code with no reason', () => {
@@ -74,7 +74,7 @@ describe( 'trackAPIError', () => {
 		expect( eventName ).toEqual( 'test-method:test-type/test-identifier/data/test-datapoint' );
 		expect( eventData.event_category ).toEqual( 'api_error' );
 		expect( eventData.event_label ).toEqual( 'test-error-message (code: test-error-code)' );
-		expect( eventData.event_value ).toEqual( 'test-error-code' );
+		expect( eventData.value ).toEqual( 'test-error-code' );
 	} );
 
 	it( 'tracks API error message & code with no data', () => {
@@ -94,7 +94,7 @@ describe( 'trackAPIError', () => {
 		expect( eventName ).toEqual( 'test-method:test-type/test-identifier/data/test-datapoint' );
 		expect( eventData.event_category ).toEqual( 'api_error' );
 		expect( eventData.event_label ).toEqual( 'test-error-message (code: test-error-code)' );
-		expect( eventData.event_value ).toEqual( 'test-error-code' );
+		expect( eventData.value ).toEqual( 'test-error-code' );
 	} );
 
 	it( "doesn't track excluded error codes", () => {

diff --git a/assets/js/util/tracking/createTrackEvent.js b/assets/js/util/tracking/createTrackEvent.js
@@ -22,13 +22,13 @@ export default function createTrackEvent( config, dataLayerTarget, _global ) {
 	 *
 	 * @since 1.3.0
 	 *
-	 * @param {string} eventCategory The event category. Required.
-	 * @param {string} eventName     The event category. Required.
-	 * @param {string} eventLabel    The event category. Optional.
-	 * @param {string} eventValue    The event category. Optional.
+	 * @param {string} category The category of the event.
+	 * @param {string} action   The action name of the event.
+	 * @param {string} [label]  Optional. The label of the event.
+	 * @param {number} [value]  Optional. A non-negative integer that will appear as the event value.
 	 * @return {Promise} Promise that always resolves.
 	 */
-	return async function trackEvent( eventCategory, eventName, eventLabel = '', eventValue = '' ) {
+	return async function trackEvent( category, action, label, value ) {
 		const {
 			isFirstAdmin,
 			referenceSiteURL,
@@ -48,9 +48,9 @@ export default function createTrackEvent( config, dataLayerTarget, _global ) {
 
 		const eventData = {
 			send_to: trackingID,
-			event_category: eventCategory,
-			event_label: eventLabel,
-			event_value: eventValue,
+			event_category: category,
+			event_label: label,
+			value,
 			dimension1: referenceSiteURL,
 			dimension2: isFirstAdmin ? 'true' : 'false',
 			dimension3: userIDHash,
@@ -63,11 +63,11 @@ export default function createTrackEvent( config, dataLayerTarget, _global ) {
 			// tracking should not result in user-facing errors. It will just
 			// trigger a console warning.
 			const failTimeout = setTimeout( () => {
-				global.console.warn( `Tracking event "${ eventName }" (category "${ eventCategory }") took too long to fire.` );
+				global.console.warn( `Tracking event "${ action }" (category "${ category }") took too long to fire.` );
 				resolve();
 			}, 1000 );
 
-			dataLayerPush( 'event', eventName, {
+			dataLayerPush( 'event', action, {
 				...eventData,
 				event_callback: () => {
 					clearTimeout( failTimeout );

diff --git a/assets/js/util/tracking/index.test.js b/assets/js/util/tracking/index.test.js
@@ -101,7 +101,7 @@ describe( 'trackEvent', () => {
 			send_to: config.trackingID,
 			event_category: 'category',
 			event_label: 'label',
-			event_value: 'value',
+			value: 'value',
 			dimension1: 'https://www.example.com',
 			dimension2: 'true',
 			dimension3: config.userIDHash,