Merge "Ready A/B test code for topic subscriptions"

extension.json

@@ -535,8 +535,7 @@
 		"discussiontools-replytool": 1,
 		"discussiontools-sourcemodetoolbar": 1,
 		"discussiontools-topicsubscription": 1,
-		"discussiontools-autotopicsub": 0,
-		"discussiontools-abtest2": ""
+		"discussiontools-autotopicsub": 0
 	},
 	"config": {
 		"DiscussionToolsEnable": {

includes/Hooks/HookUtils.php

@@ -151,39 +151,27 @@ class HookUtils {
 	/**
 	 * Work out the A/B test bucket for the current user
 	 *
-	 * Checks whether the user has been enrolled in the last A/B test, if any was enabled.
-	 *
-	 * If the A/B test is enabled, and the user is eligible and not enrolled, it will enroll them.
+	 * Currently this just checks whether the user is logged in, and assigns
+	 * them to a consistent bucket based on their ID.
 	 *
 	 * @param UserIdentity $user
 	 * @param string|null $feature Feature to check for (one of static::FEATURES)
 	 *  Null will check for any DT feature.
-	 * @return string 'test' if in the test group, 'control' if in the control group, or '' if they've
-	 *  never been in the test
+	 * @return string 'test' if in the test group, 'control' if in the control group, or '' if
+	 * 	they're not in the test
 	 */
-	private static function determineUserABTestBucket( UserIdentity $user, ?string $feature = null ): string {
+	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' );
-		$abstate = $optionsManager->getOption( $user, 'discussiontools-abtest2' );
 
 		if (
 			$user->isRegistered() &&
-			$feature && $abtest == $feature
+			( $feature ? ( $abtest == $feature ) : (bool)$abtest )
 		) {
-			// The A/B test is enabled, and the user is qualified to be in the
-			// test by being logged in.
-			if ( !$abstate && !$optionsManager->getOption( $user, 'discussiontools-newtopictool-opened' ) ) {
-				// Assign the user to a group. This is only being done to
-				// users who have never used the tool before, for which we're
-				// using the absence of discussiontools-newtopictool-opened.
-				$abstate = $user->getId() % 2 == 0 ? 'test' : 'control';
-				$optionsManager->setOption( $user, 'discussiontools-abtest2', $abstate );
-				$optionsManager->saveOptions( $user );
-			}
-			return $abstate;
+			return $user->getId() % 2 == 0 ? 'test' : 'control';
 		}
 		return '';
 	}

includes/Hooks/PageHooks.php

@@ -103,15 +103,7 @@ class PageHooks implements
 		$dtConfig = $this->configFactory->makeConfig( 'discussiontools' );
 
 		// Load modules if any DT feature is enabled for this user
-		if (
-			HookUtils::isFeatureEnabledForOutput( $output ) || (
-				// logged out users should get the modules when there's an a/b test
-				// client-side overrides of the enabling will occur
-				$availableForTitle &&
-				$dtConfig->get( 'DiscussionToolsABTest' ) &&
-				!$user->isRegistered()
-			)
-		) {
+		if ( HookUtils::isFeatureEnabledForOutput( $output ) ) {
 			$output->addModules( [
 				'ext.discussionTools.init'
 			] );
@@ -136,15 +128,17 @@ class PageHooks implements
 					$editor
 				);
 			}
-			$abstate = $dtConfig->get( 'DiscussionToolsABTest' ) ?
-				$this->userOptionsLookup->getOption( $user, 'discussiontools-abtest2' ) :
-				false;
-			if ( $abstate ) {
-				$output->addJsConfigVars(
-					'wgDiscussionToolsABTestBucket',
-					$abstate
-				);
-			}
+		}
+
+		// This doesn't involve any DB checks, and so we can put it on every
+		// page to make it easy to pick for logging in WikiEditor. If this
+		// becomes not-cheap, move it elsewhere.
+		$abstate = HookUtils::determineUserABTestBucket( $user );
+		if ( $abstate ) {
+			$output->addJsConfigVars(
+				'wgDiscussionToolsABTestBucket',
+				$abstate
+			);
 		}
 
 		// Replace the action=edit&section=new form with the new topic tool.

includes/Hooks/PreferenceHooks.php

@@ -118,9 +118,6 @@ class PreferenceHooks implements
 		$preferences['discussiontools-showadvanced'] = [
 			'type' => 'api',
 		];
-		$preferences['discussiontools-abtest2'] = [
-			'type' => 'api',
-		];
 		$preferences['discussiontools-newtopictool-opened'] = [
 			'type' => 'api',
 		];

modules/dt.init.js

@@ -11,34 +11,8 @@ mw.dt.initState = {
 	firstLoad: true
 };
 
-// New Topic A/B test for logged out users:
-var tokenData = mw.storage.getObject( 'DTNewTopicABToken' );
-if ( tokenData && tokenData.expires < Date.now() ) {
-	mw.storage.remove( 'DTNewTopicABToken' );
-	tokenData = null;
-}
-if ( mw.user.isAnon() && mw.config.get( 'wgDiscussionToolsABTest' ) ) {
-	if ( !tokenData ) {
-		tokenData = {
-			token: mw.user.generateRandomSessionId(),
-			// 90 days
-			expires: Date.now() + 90 * 24 * 60 * 60 * 1000
-		};
-		mw.storage.setObject( 'DTNewTopicABToken', tokenData );
-	}
-	mw.config.set( 'wgDiscussionToolsAnonymousUserId', tokenData.token );
-	var anonid = parseInt( tokenData.token.slice( 0, 8 ), 16 );
-	var abstate = anonid % 2 === 0 ? 'test' : 'control';
-	mw.config.set( 'wgDiscussionToolsABTestBucket', abstate );
-	var featuresEnabled = mw.config.get( 'wgDiscussionToolsFeaturesEnabled' ) || {};
-	if ( abstate === 'test' ) {
-		$( document.body ).addClass( 'ext-discussiontools-newtopictool-enabled' );
-		featuresEnabled.newtopictool = true;
-	} else {
-		$( document.body ).removeClass( 'ext-discussiontools-newtopictool-enabled' );
-		featuresEnabled.newtopictool = false;
-	}
-}
+// Cleaning up anonymous A/B test token; remove later.
+mw.storage.remove( 'DTNewTopicABToken' );
 
 if ( url.searchParams.get( 'dtrepliedto' ) ) {
 	// If we had to reload the page to highlight the new comment, extract that data from the URL and