Merge "Refactor TOC A/B test to bucket users on backend"
This commit is contained in:
commit
177e57b794
|
@ -206,6 +206,16 @@ final class Constants {
|
||||||
*/
|
*/
|
||||||
public const FEATURE_LANGUAGE_ALERT_IN_SIDEBAR = 'LanguageAlertInSidebar';
|
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
|
* @var string
|
||||||
*/
|
*/
|
||||||
|
|
|
@ -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;
|
||||||
|
}
|
||||||
|
}
|
|
@ -557,14 +557,18 @@ class Hooks implements
|
||||||
}
|
}
|
||||||
|
|
||||||
$classes = [];
|
$classes = [];
|
||||||
$isTocABTestEnabled = $sk->isTOCABTestEnabled();
|
|
||||||
|
|
||||||
if (
|
if (
|
||||||
|
$sk->isTOCABTestEnabled() &&
|
||||||
$sk->isTableOfContentsVisibleInSidebar() &&
|
$sk->isTableOfContentsVisibleInSidebar() &&
|
||||||
$isTocABTestEnabled
|
!$sk->getUser()->isAnon()
|
||||||
) {
|
) {
|
||||||
|
$userBucket = !$sk->isUserInTocTreatmentBucket()
|
||||||
|
? 'control'
|
||||||
|
: 'treatment';
|
||||||
$experimentConfig = $config->get( Constants::CONFIG_WEB_AB_TEST_ENROLLMENT );
|
$experimentConfig = $config->get( Constants::CONFIG_WEB_AB_TEST_ENROLLMENT );
|
||||||
$classes[] = $experimentConfig[ 'name' ];
|
$experimentName = $experimentConfig[ 'name' ];
|
||||||
|
$classes[] = $experimentName;
|
||||||
|
$classes[] = "$experimentName-$userBucket";
|
||||||
}
|
}
|
||||||
|
|
||||||
return $classes;
|
return $classes;
|
||||||
|
|
|
@ -27,6 +27,7 @@ use MediaWiki\Skins\Vector\Constants;
|
||||||
use MediaWiki\Skins\Vector\FeatureManagement\FeatureManager;
|
use MediaWiki\Skins\Vector\FeatureManagement\FeatureManager;
|
||||||
use MediaWiki\Skins\Vector\FeatureManagement\Requirements\DynamicConfigRequirement;
|
use MediaWiki\Skins\Vector\FeatureManagement\Requirements\DynamicConfigRequirement;
|
||||||
use MediaWiki\Skins\Vector\FeatureManagement\Requirements\OverridableConfigRequirement;
|
use MediaWiki\Skins\Vector\FeatureManagement\Requirements\OverridableConfigRequirement;
|
||||||
|
use MediaWiki\Skins\Vector\FeatureManagement\Requirements\TableOfContentsTreatmentRequirement;
|
||||||
|
|
||||||
return [
|
return [
|
||||||
Constants::SERVICE_FEATURE_MANAGER => static function ( MediaWikiServices $services ) {
|
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;
|
return $featureManager;
|
||||||
}
|
}
|
||||||
];
|
];
|
||||||
|
|
|
@ -2,8 +2,6 @@
|
||||||
|
|
||||||
namespace MediaWiki\Skins\Vector;
|
namespace MediaWiki\Skins\Vector;
|
||||||
|
|
||||||
use MediaWiki\MediaWikiServices;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @ingroup Skins
|
* @ingroup Skins
|
||||||
* @package Vector
|
* @package Vector
|
||||||
|
@ -37,8 +35,17 @@ class SkinVector22 extends SkinVector {
|
||||||
$experimentConfig = $this->getConfig()->get( Constants::CONFIG_WEB_AB_TEST_ENROLLMENT );
|
$experimentConfig = $this->getConfig()->get( Constants::CONFIG_WEB_AB_TEST_ENROLLMENT );
|
||||||
|
|
||||||
return $experimentConfig['name'] === self::TOC_AB_TEST_NAME &&
|
return $experimentConfig['name'] === self::TOC_AB_TEST_NAME &&
|
||||||
$experimentConfig['enabled'] &&
|
$experimentConfig['enabled'];
|
||||||
MediaWikiServices::getInstance()->hasService( Constants::WEB_AB_TEST_ARTICLE_ID_FACTORY_SERVICE );
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 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 );
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -1,7 +1,12 @@
|
||||||
.skin-vector-toc-experiment-control .mw-table-of-contents-container,
|
.skin-vector-toc-experiment-control .mw-table-of-contents-container,
|
||||||
.skin-vector-toc-experiment-treatment #toc,
|
.skin-vector-toc-experiment-control #vector-toc-collapsed-button,
|
||||||
.skin-vector-toc-experiment-unsampled .mw-table-of-contents-container {
|
body:not( .skin-vector-toc-experiment-control ) #toc {
|
||||||
// This trumps any layout rules e.g. vector-layout-grid
|
// This trumps any layout rules e.g. vector-layout-grid
|
||||||
/* stylelint-disable-next-line declaration-no-important */
|
/* stylelint-disable-next-line declaration-no-important */
|
||||||
display: none !important;
|
display: none !important;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// T313435 Show legacy toc for control group.
|
||||||
|
.skin-vector-toc-experiment-control #toc {
|
||||||
|
display: table;
|
||||||
|
}
|
||||||
|
|
|
@ -215,6 +215,7 @@ const main = () => {
|
||||||
document.body.classList.contains( ABTestConfig.name ) &&
|
document.body.classList.contains( ABTestConfig.name ) &&
|
||||||
// eslint-disable-next-line compat/compat
|
// eslint-disable-next-line compat/compat
|
||||||
window.URLSearchParams &&
|
window.URLSearchParams &&
|
||||||
|
!mw.user.isAnon() &&
|
||||||
initExperiment( ABTestConfig );
|
initExperiment( ABTestConfig );
|
||||||
const isInTreatmentBucket = !!experiment && experiment.isInTreatmentBucket();
|
const isInTreatmentBucket = !!experiment && experiment.isInTreatmentBucket();
|
||||||
|
|
||||||
|
|
|
@ -123,11 +123,3 @@
|
||||||
.client-js body.rtl .sidebar-toc .sidebar-toc-toggle {
|
.client-js body.rtl .sidebar-toc .sidebar-toc-toggle {
|
||||||
transform: rotate( 90deg );
|
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;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
|
@ -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 );
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue