Refactor TOC A/B test to bucket users on backend

- Include temporary feature requirement for TOC A/B test.
  - Assumes 100% of logged-in users with even/odd user ids
  being assigned to treatment/control buckets respectively.
  - Sampling rates passed in by config are not considered
  during bucketing.
- Update hook for adding needed TOC A/B test body classes.
- Add test for temp feature.

Note: the temporary feature requirement and associated hooks
should be removed once the 2nd TOC A/B test concludes.

Bug: T313435
Change-Id: If9c75235614af289cd50182baab29bec3155eb81

includes/Constants.php

@@ -206,6 +206,16 @@ final class Constants {
 	 */
 	public const FEATURE_LANGUAGE_ALERT_IN_SIDEBAR = 'LanguageAlertInSidebar';
 
+	/**
+	 * @var string
+	 */
+	public const FEATURE_TABLE_OF_CONTENTS = 'TableOfContents';
+
+	/**
+	 * @var string
+	 */
+	public const REQUIREMENT_TABLE_OF_CONTENTS = 'TableOfContents';
+
 	/**
 	 * @var string
 	 */

includes/FeatureManagement/Requirements/TableOfContentsTreatmentRequirement.php

@@ -0,0 +1,84 @@
+<?php
+
+namespace MediaWiki\Skins\Vector\FeatureManagement\Requirements;
+
+use CentralIdLookup;
+use Config;
+use MediaWiki\Skins\Vector\Constants;
+use MediaWiki\Skins\Vector\FeatureManagement\Requirement;
+use User;
+
+/**
+ * Checks whether or not sticky Table of Contents should be shown.
+ *
+ * @unstable
+ *
+ * @package Vector\FeatureManagement\Requirements
+ * @internal
+ */
+final class TableOfContentsTreatmentRequirement implements Requirement {
+	/**
+	 * @var Config
+	 */
+	private $config;
+
+	/**
+	 * @var User
+	 */
+	private $user;
+
+	/**
+	 * @var CentralIdLookup
+	 */
+	private $centralIdLookup;
+
+	/**
+	 * @param Config $config
+	 * @param User $user
+	 * @param CentralIdLookup|null $centralIdLookup
+	 */
+	public function __construct(
+		Config $config,
+		User $user,
+		?CentralIdLookup $centralIdLookup
+	) {
+		$this->config = $config;
+		$this->user = $user;
+		$this->centralIdLookup = $centralIdLookup;
+	}
+
+	/**
+	 * @inheritDoc
+	 */
+	public function getName(): string {
+		return Constants::REQUIREMENT_TABLE_OF_CONTENTS;
+	}
+
+	/**
+	 * If A/B test is enabled check whether the user is logged in and bucketed.
+	 *
+	 * @inheritDoc
+	 * @throws \ConfigException
+	 */
+	public function isMet(): bool {
+		$currentAbTest = $this->config->get( Constants::CONFIG_WEB_AB_TEST_ENROLLMENT );
+		if ( $currentAbTest['enabled'] && $this->user->isRegistered() ) {
+			$id = null;
+			if ( $this->centralIdLookup ) {
+				$id = $this->centralIdLookup->centralIdFromLocalUser( $this->user );
+			}
+
+			// $id will be 0 if the central ID lookup failed.
+			if ( !$id ) {
+				$id = $this->user->getId();
+			}
+
+			// This assume 100% sampling of logged-in users with roughly half
+			// in control or treatment buckets based on even or odd user ids.
+			// This does not cover unsampled users nor does it consider the
+			// sampling rates of any given bucket passed in via config.
+			return $id % 2 === 0;
+		}
+		return false;
+	}
+}

includes/Hooks.php

@@ -557,14 +557,18 @@ class Hooks implements
 		}
 
 		$classes = [];
-		$isTocABTestEnabled = $sk->isTOCABTestEnabled();
-
 		if (
+			$sk->isTOCABTestEnabled() &&
 			$sk->isTableOfContentsVisibleInSidebar() &&
-			$isTocABTestEnabled
+			!$sk->getUser()->isAnon()
 		) {
+			$userBucket = !$sk->isUserInTocTreatmentBucket()
+				? 'control'
+				: 'treatment';
 			$experimentConfig = $config->get( Constants::CONFIG_WEB_AB_TEST_ENROLLMENT );
-			$classes[] = $experimentConfig[ 'name' ];
+			$experimentName = $experimentConfig[ 'name' ];
+			$classes[] = $experimentName;
+			$classes[] = "$experimentName-$userBucket";
 		}
 
 		return $classes;

includes/ServiceWiring.php

@@ -27,6 +27,7 @@ use MediaWiki\Skins\Vector\Constants;
 use MediaWiki\Skins\Vector\FeatureManagement\FeatureManager;
 use MediaWiki\Skins\Vector\FeatureManagement\Requirements\DynamicConfigRequirement;
 use MediaWiki\Skins\Vector\FeatureManagement\Requirements\OverridableConfigRequirement;
+use MediaWiki\Skins\Vector\FeatureManagement\Requirements\TableOfContentsTreatmentRequirement;
 
 return [
 	Constants::SERVICE_FEATURE_MANAGER => static function ( MediaWikiServices $services ) {
@@ -186,6 +187,25 @@ return [
 			]
 		);
 
+		// T313435 Feature: Table of Contents
+		// Temporary - remove after TOC A/B test is finished.
+		// ================================
+		$featureManager->registerRequirement(
+			new TableOfContentsTreatmentRequirement(
+				$services->getMainConfig(),
+				$context->getUser(),
+				$services->getCentralIdLookupFactory()->getNonLocalLookup()
+			)
+		);
+
+		$featureManager->registerFeature(
+			Constants::FEATURE_TABLE_OF_CONTENTS,
+			[
+				Constants::REQUIREMENT_FULLY_INITIALISED,
+				Constants::REQUIREMENT_TABLE_OF_CONTENTS,
+			]
+		);
+
 		return $featureManager;
 	}
 ];

includes/SkinVector22.php

@@ -2,8 +2,6 @@
 
 namespace MediaWiki\Skins\Vector;
 
-use MediaWiki\MediaWikiServices;
-
 /**
  * @ingroup Skins
  * @package Vector
@@ -37,8 +35,17 @@ class SkinVector22 extends SkinVector {
 		$experimentConfig = $this->getConfig()->get( Constants::CONFIG_WEB_AB_TEST_ENROLLMENT );
 
 		return $experimentConfig['name'] === self::TOC_AB_TEST_NAME &&
-			$experimentConfig['enabled'] &&
-			MediaWikiServices::getInstance()->hasService( Constants::WEB_AB_TEST_ARTICLE_ID_FACTORY_SERVICE );
+			$experimentConfig['enabled'];
+	}
+
+	/**
+	 * Check whether the user is bucketed in the treatment group for TOC.
+	 *
+	 * @return bool
+	 */
+	public function isUserInTocTreatmentBucket(): bool {
+		$featureManager = VectorServices::getFeatureManager();
+		return $featureManager->isFeatureEnabled( Constants::FEATURE_TABLE_OF_CONTENTS );
 	}
 
 	/**

resources/skins.vector.AB.styles.less

@@ -1,7 +1,12 @@
 .skin-vector-toc-experiment-control .mw-table-of-contents-container,
-.skin-vector-toc-experiment-treatment #toc,
-.skin-vector-toc-experiment-unsampled .mw-table-of-contents-container {
+.skin-vector-toc-experiment-control #vector-toc-collapsed-button,
+body:not( .skin-vector-toc-experiment-control ) #toc {
 	// This trumps any layout rules e.g. vector-layout-grid
 	/* stylelint-disable-next-line declaration-no-important */
 	display: none !important;
 }
+
+// T313435 Show legacy toc for control group.
+.skin-vector-toc-experiment-control #toc {
+	display: table;
+}

resources/skins.vector.es6/main.js

@@ -209,6 +209,7 @@ const main = () => {
 		document.body.classList.contains( ABTestConfig.name ) &&
 		// eslint-disable-next-line compat/compat
 		window.URLSearchParams &&
+		!mw.user.isAnon() &&
 		initExperiment( ABTestConfig );
 	const isInTreatmentBucket = !!experiment && experiment.isInTreatmentBucket();
 

resources/skins.vector.styles/components/TableOfContents.less

@@ -123,11 +123,3 @@
 .client-js body.rtl .sidebar-toc .sidebar-toc-toggle {
 	transform: rotate( 90deg );
 }
-
-// T300975 following media query for TOC experiment treatment
-// class can be removed once associated A/B test is over.
-@media ( max-width: @max-width-tablet ) {
-	.skin-vector-toc-experiment-treatment #toc {
-		display: table;
-	}
-}

tests/phpunit/unit/FeatureManagement/Requirements/TableOfContentsTreatmentRequirementTest.php

@@ -0,0 +1,105 @@
+<?php
+
+namespace MediaWiki\Skins\Vector\Tests\Unit\FeatureManagement\Requirements;
+
+use CentralIdLookup;
+use HashConfig;
+use MediaWiki\Skins\Vector\Constants;
+use MediaWiki\Skins\Vector\FeatureManagement\Requirements\TableOfContentsTreatmentRequirement;
+use User;
+
+/**
+ * @group Vector
+ * @group FeatureManagement
+ * @coversDefaultClass \MediaWiki\Skins\Vector\FeatureManagement\Requirements\TableOfContentsTreatmentRequirement
+ */
+class TableOfContentsTreatmentRequirementTest extends \MediaWikiUnitTestCase {
+
+	public function providerTableOfContentsTreatmentRequirement() {
+		return [
+			[
+				// is A-B test enabled
+				false,
+				// logged-in user with even ID
+				10,
+				// use central id lookup?
+				true,
+				false,
+				'If nothing enabled, nobody sees new treatment'
+			],
+			[
+				// is A-B test enabled
+				true,
+				// note 0 = anon user
+				0,
+				// use central id lookup?
+				false,
+				false,
+				'If test enabled, anon does not see new treatment'
+			],
+			[
+				// is A-B test enabled
+				true,
+				// logged-in user with even ID
+				108,
+				// use central id lookup?
+				true,
+				true,
+				'If test enabled, logged-in user with even ID sees new treatment'
+			],
+			[
+				// is A-B test enabled
+				true,
+				// logged-in user with odd ID
+				7,
+				// use central id lookup?
+				true,
+				false,
+				'If test enabled, logged-in user with odd ID does not see new treatment'
+			],
+		];
+	}
+
+	/**
+	 * @covers ::isMet
+	 * @dataProvider providerTableOfContentsTreatmentRequirement
+	 * @param bool $abValue
+	 * @param int $userId
+	 * @param bool $useCentralIdLookup
+	 * @param bool $expected
+	 * @param string $msg
+	 */
+	public function testTableOfContentsTreatmentRequirement(
+		$abValue, $userId, $useCentralIdLookup, $expected, $msg
+	) {
+		$config = new HashConfig( [
+			Constants::CONFIG_WEB_AB_TEST_ENROLLMENT => [
+				'name' => 'skin-vector-toc-experiment',
+				'enabled' => $abValue,
+				'buckets' => [
+					'control' => [
+						'samplingRate' => 0.5,
+					],
+					'treatment' => [
+						'samplingRate' => 0.5,
+					]
+				]
+			],
+		] );
+
+		$user = $this->createMock( User::class );
+		$user->method( 'isRegistered' )->willReturn( $userId !== 0 );
+		$user->method( 'getID' )->willReturn( $userId );
+
+		$centralIdLookup = $this->createMock( CentralIdLookup::class );
+		$centralIdLookup->method( 'centralIdFromLocalUser' )->willReturn( $userId );
+
+		$requirement = new TableOfContentsTreatmentRequirement(
+			$config,
+			$user,
+			$useCentralIdLookup ? $centralIdLookup : null
+		);
+
+		$this->assertSame( $expected, $requirement->isMet(), $msg );
+	}
+}