From 71d2e76d7f8c10c29a9259999603c755eda5cecb Mon Sep 17 00:00:00 2001
From: David Lynch <dlynch@wikimedia.org>
Date: Mon, 8 May 2023 01:30:21 -0500
Subject: [PATCH] Update a/b test code for visual enhancements a/b test

Strip it out from applying to logged out users and make the test work
for multiple features

Bug: T333715
Change-Id: Id15a8a99c2ea8e6fc14fc83baf2ed6ebaaf754c8
---
 extension.json               |  2 +-
 includes/Hooks/HookUtils.php | 25 ++++++++-----------------
 includes/Hooks/PageHooks.php |  7 +------
 3 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/extension.json b/extension.json
index 93a189fd0..cf41db37a 100644
--- a/extension.json
+++ b/extension.json
@@ -540,7 +540,7 @@
 		},
 		"DiscussionToolsABTest": {
 			"value": false,
-			"description": "A/B test DiscussionTools features for logged in users. false, 'replytool', 'newtopictool', or 'mobile'"
+			"description": "A/B test DiscussionTools features for logged in users. false, any valid feature string for an option below, or an array thereof"
 		},
 		"DiscussionToolsEnableMobile": {
 			"value": true,
diff --git a/includes/Hooks/HookUtils.php b/includes/Hooks/HookUtils.php
index ff9ef0b9e..d3ac64e19 100644
--- a/includes/Hooks/HookUtils.php
+++ b/includes/Hooks/HookUtils.php
@@ -300,27 +300,18 @@ class HookUtils {
 	 */
 	public static function determineUserABTestBucket( UserIdentity $user, ?string $feature = null ): string {
 		$services = MediaWikiServices::getInstance();
-		$optionsManager = $services->getUserOptionsManager();
 		$dtConfig = $services->getConfigFactory()->makeConfig( 'discussiontools' );
 
 		$abtest = $dtConfig->get( 'DiscussionToolsABTest' );
+		if ( !$abtest ) {
+			return '';
+		}
 
-		if ( $feature ? ( $abtest == $feature ) : (bool)$abtest ) {
-			if ( $user->isRegistered() ) {
-				return $user->getId() % 2 == 0 ? 'test' : 'control';
-			}
-			// logged out
-			$req = RequestContext::getMain()->getRequest();
-			$cookie = $req->getCookie( 'DTAB', '' );
-			if ( $cookie ) {
-				return $cookie;
-			}
-			// we just want to remember this across all calls in this request
-			static $bucket = false;
-			if ( !$bucket ) {
-				$bucket = rand( 0, 1 ) <= 0.5 ? 'test' : 'control';
-			}
-			return $bucket;
+		if (
+			( $feature ? in_array( $feature, (array)$abtest ) : (bool)$abtest ) &&
+			$user->isRegistered()
+		) {
+			return $user->getId() % 2 == 0 ? 'test' : 'control';
 		}
 		return '';
 	}
diff --git a/includes/Hooks/PageHooks.php b/includes/Hooks/PageHooks.php
index dcc161f3a..a36bd5e0f 100644
--- a/includes/Hooks/PageHooks.php
+++ b/includes/Hooks/PageHooks.php
@@ -109,12 +109,7 @@ class PageHooks implements
 		}
 
 		// Load modules if any DT feature is enabled for this user
-		if (
-			HookUtils::isFeatureEnabledForOutput( $output ) ||
-			// If there's an a/b test we need to include the JS for unregistered users just so
-			// we can make sure we store the bucket
-			( $this->config->get( 'DiscussionToolsABTest' ) && !$user->isRegistered() )
-		) {
+		if ( HookUtils::isFeatureEnabledForOutput( $output ) ) {
 			$output->addModules( [
 				'ext.discussionTools.init'
 			] );