From e27cbf2a498badfc015a7301c67a17791aa06984 Mon Sep 17 00:00:00 2001
From: Jan Drewniak <jan.drewniak@gmail.com>
Date: Mon, 2 Jul 2018 15:15:04 +0200
Subject: [PATCH] Reading depth hook should send additional sampling bucket
 parameter

The hook that enables the Reading depth test should send an
additional paramter that specifies which test bucket the hook
being is calling from.

Bug: T191532
Change-Id: Ifd9f43220c476ece8a0c0cee46b62b58a717c616
---
 README.md                                     | 10 ++++----
 resources/skins.minerva.scripts/AB.js         | 24 +++++++++----------
 .../skins.minerva.scripts/cleanuptemplates.js | 19 +++++++++++----
 tests/qunit/skins.minerva.scripts/test_AB.js  | 12 +++++++---
 4 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/README.md b/README.md
index ff0bfa4..e40b3d4 100644
--- a/README.md
+++ b/README.md
@@ -122,11 +122,11 @@ Group membership can be debugged from the console via:
 
 ```js
   const AB = mw.mobileFrontend.require('skins.minerva.scripts/AB')
-  new AB(
-    'WME.PageIssuesAB',
-    mw.config.get( 'wgMinervaABSamplingRate', 0 ),
-    mw.user.sessionId()
-  ).getBucket()
+  new AB({
+    testName: 'WME.PageIssuesAB',
+    samplingRate: mw.config.get( 'wgMinervaABSamplingRate', 0 ),
+    sessionId: mw.user.sessionId()
+  }).getBucket()
 ```
 
 And since session ID is an input in calculating the group, reassignment occurs
diff --git a/resources/skins.minerva.scripts/AB.js b/resources/skins.minerva.scripts/AB.js
index 701a763..ec3312e 100644
--- a/resources/skins.minerva.scripts/AB.js
+++ b/resources/skins.minerva.scripts/AB.js
@@ -11,14 +11,21 @@
 	/**
 	 * Buckets users based on params and exposes an `isEnabled` and `getBucket` method.
 	 *
-	 * @param {string} testName name of the AB-test.
-	 * @param {number} samplingRate sampling rate for the AB-test.
-	 * @param {number} sessionId session ID for user bucketing.
+	 * @param {Object}   config configuration object for AB test.
+	 * @param {string}   config.testName
+	 * @param {number}   config.samplingRate sampling rate for the AB-test.
+	 * @param {number}   config.sessionId session ID for user bucketing
+	 * @param {function} [config.onABStart] function that triggers event-logging when user is either
+	 *                                      in bucket A or B.
 	 * @constructor
 	 */
-	function AB( testName, samplingRate, sessionId ) {
+	function AB( config ) {
 
 		var CONTROL_BUCKET = 'control',
+			testName = config.testName,
+			samplingRate = config.samplingRate,
+			sessionId = config.sessionId,
+			onABStart = config.onABStart || function () {},
 			test = {
 				name: testName,
 				enabled: !!samplingRate,
@@ -28,13 +35,6 @@
 					B: samplingRate / 2
 				}
 			};
-		/**
-		 * Starts the AB-test and enters the user into the Reading Depth test.
- 		 */
-		function startABTest() {
-			// See: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/WikimediaEvents/+/437686/
-			mw.track( 'wikimedia.event.ReadingDepthSchema.enable' );
-		}
 
 		/**
 		 * Gets the users AB-test bucket
@@ -61,7 +61,7 @@
 		 */
 		function init() {
 			if ( isEnabled() ) {
-				startABTest();
+				onABStart( getBucket() );
 			}
 		}
 
diff --git a/resources/skins.minerva.scripts/cleanuptemplates.js b/resources/skins.minerva.scripts/cleanuptemplates.js
index dd8d8a5..0686b34 100644
--- a/resources/skins.minerva.scripts/cleanuptemplates.js
+++ b/resources/skins.minerva.scripts/cleanuptemplates.js
@@ -6,11 +6,20 @@
 		NS_MAIN = 0,
 		NS_TALK = 1,
 		NS_CATEGORY = 14,
-		isInGroupB = new AB(
-			'WME.PageIssuesAB',
-			mw.config.get( 'wgMinervaABSamplingRate', 0 ),
-			mw.user.sessionId()
-		).getBucket() === 'B',
+		isInGroupB = new AB( {
+			testName: 'WME.PageIssuesAB',
+			samplingRate: mw.config.get( 'wgMinervaABSamplingRate', 0 ),
+			sessionId: mw.user.sessionId(),
+			onABStart: function ( bucket ) {
+				// See: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/WikimediaEvents/+/437686/
+				var READING_DEPTH_BUCKET_KEYS = {
+						A: 'page-issues-a_sample',
+						B: 'page-issues-b_sample'
+					},
+					readingDepthBucket = READING_DEPTH_BUCKET_KEYS[ bucket ];
+				mw.track( 'wikimedia.event.ReadingDepthSchema.enable', readingDepthBucket );
+			}
+		} ).getBucket() === 'B',
 		Icon = M.require( 'mobile.startup/Icon' );
 
 	( function () {
diff --git a/tests/qunit/skins.minerva.scripts/test_AB.js b/tests/qunit/skins.minerva.scripts/test_AB.js
index 755b72b..ee3c79d 100644
--- a/tests/qunit/skins.minerva.scripts/test_AB.js
+++ b/tests/qunit/skins.minerva.scripts/test_AB.js
@@ -1,8 +1,12 @@
 ( function ( M ) {
 
 	var AB = M.require( 'skins.minerva.scripts/AB' ),
-		aBName = 'WME.MinervaABTest',
-		samplingRate = 0.5;
+		util = M.require( 'mobile.startup/util' ),
+		defaultConfig = {
+			testName: 'WME.MinervaABTest',
+			samplingRate: 0.5,
+			sessionId: mw.user.generateRandomSessionId()
+		};
 
 	QUnit.module( 'Minerva AB-test' );
 
@@ -14,10 +18,12 @@
 			},
 			maxUsers = 1000,
 			bucketingTest,
+			config,
 			i;
 
 		for ( i = 0; i < maxUsers; i++ ) {
-			bucketingTest = new AB( aBName, samplingRate, mw.user.generateRandomSessionId() );
+			config = util.extend( {}, defaultConfig, { sessionId: mw.user.generateRandomSessionId() } );
+			bucketingTest = new AB( config );
 			userBuckets[ bucketingTest.getBucket() ] += 1;
 		}