wikimedia/mediawiki-skins-Vector
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/skins/Vector.git synced 2024-11-24 15:53:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge "Remove A/B testing element from OverridableConfigRequirement"

Browse source
This commit is contained in:
jenkins-bot 2023-04-19 11:24:23 +00:00 committed by Gerrit Code Review
parent 1438dafcf7 a76e198523
commit 7407c48274
3 changed files with 6 additions and 259 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
includes/FeatureManagement/Requirements/OverridableConfigRequirement.php
View file

@ -21,7 +21,6 @@
namespace MediaWiki\Skins\Vector\FeatureManagement\Requirements;
use CentralIdLookup;
use Config;
use MediaWiki\Skins\Vector\FeatureManagement\Requirement;
use User;
@ -37,7 +36,6 @@ use WebRequest;
* $config,
* $user,
* $request,
* $centralIdLookup,
* MainConfigNames::Sitename,
* 'requirementName',
* 'overrideName',
@ -82,11 +80,6 @@ final class OverridableConfigRequirement implements Requirement {
*/
private $request;
/**
* @var CentralIdLookup|null
*/
private $centralIdLookup;
/**
* @var string
*/
@ -102,11 +95,6 @@ final class OverridableConfigRequirement implements Requirement {
*/
private $overrideName;
/**
* @var string|null
*/
private $configTestName;
/**
* This constructor accepts all dependencies needed to determine whether
* the overridable config is enabled for the current user and request.
@ -114,31 +102,25 @@ final class OverridableConfigRequirement implements Requirement {
* @param Config $config
* @param User $user
* @param WebRequest $request
* @param CentralIdLookup|null $centralIdLookup
* @param string $configName Any `Config` key. This name is used to query `$config` state.
* @param string $requirementName The name of the requirement presented to FeatureManager.
* @param string|null $overrideName The name of the override presented to FeatureManager, i.e. query parameter.
* When not set defaults to lowercase version of config key.
* @param string|null $configTestName The name of the AB test config presented to FeatureManager if applicable.
*/
public function __construct(
Config $config,
User $user,
WebRequest $request,
?CentralIdLookup $centralIdLookup,
string $configName,
string $requirementName,
?string $overrideName = null,
?string $configTestName = null
?string $overrideName = null
) {
$this->config = $config;
$this->user = $user;
$this->request = $request;
$this->centralIdLookup = $centralIdLookup;
$this->configName = $configName;
$this->requirementName = $requirementName;
$this->overrideName = $overrideName === null ? strtolower( $configName ) : $overrideName;
$this->configTestName = $configTestName;
}
/**
@ -161,25 +143,6 @@ final class OverridableConfigRequirement implements Requirement {
return $this->request->getBool( $this->overrideName );
}
// Check if there's config for AB testing.
if (
!empty( $this->configTestName ) &&
(bool)$this->config->get( $this->configTestName ) &&
$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();
}
return $id % 2 === 0;
}
// If AB test is not enabled, fallback to checking config state.
$thisConfig = $this->config->get( $this->configName );

6
includes/ServiceWiring.php
View file

@ -58,7 +58,6 @@ return [
$services->getMainConfig(),
$context->getUser(),
$context->getRequest(),
$services->getCentralIdLookupFactory()->getNonLocalLookup(),
Constants::CONFIG_KEY_LANGUAGE_IN_HEADER,
Constants::REQUIREMENT_LANGUAGE_IN_HEADER,
null
@ -92,7 +91,6 @@ return [
$config,
$context->getUser(),
$context->getRequest(),
$services->getCentralIdLookupFactory()->getNonLocalLookup(),
Constants::CONFIG_KEY_LANGUAGE_IN_HEADER,
$requirementName
)
@ -115,7 +113,6 @@ return [
$services->getMainConfig(),
$context->getUser(),
$context->getRequest(),
null,
Constants::CONFIG_LANGUAGE_IN_MAIN_PAGE_HEADER,
Constants::REQUIREMENT_LANGUAGE_IN_MAIN_PAGE_HEADER
)
@ -143,7 +140,6 @@ return [
$services->getMainConfig(),
$context->getUser(),
$context->getRequest(),
null,
Constants::CONFIG_LANGUAGE_ALERT_IN_SIDEBAR,
Constants::REQUIREMENT_LANGUAGE_ALERT_IN_SIDEBAR
)
@ -165,7 +161,6 @@ return [
$services->getMainConfig(),
$context->getUser(),
$context->getRequest(),
null,
Constants::CONFIG_STICKY_HEADER,
Constants::REQUIREMENT_STICKY_HEADER
)
@ -291,7 +286,6 @@ return [
$services->getMainConfig(),
$context->getUser(),
$context->getRequest(),
null,
Constants::CONFIG_ZEBRA_DESIGN,
Constants::REQUIREMENT_ZEBRA_DESIGN,
Constants::REQUIREMENT_ZEBRA_AB_TEST

220
tests/phpunit/unit/FeatureManagement/Requirements/OverridableConfigRequirementTest.php
View file

@ -20,7 +20,6 @@
namespace MediaWiki\Skins\Vector\Tests\Unit\FeatureManagement\Requirements;
use CentralIdLookup;
use HashConfig;
use MediaWiki\Skins\Vector\Constants;
use MediaWiki\Skins\Vector\FeatureManagement\Requirements\OverridableConfigRequirement;
@ -42,16 +41,10 @@ class OverridableConfigRequirementTest extends \MediaWikiUnitTestCase {
'logged_in' => false,
'logged_out' => false,
],
// is A-B test enabled
false,
// note 0 = anon user
0,
// use central id lookup?
false,
// `languageinheader` query param
null,
// AB test name
null,
false,
'If nothing enabled, nobody gets new treatment'
],
@ -61,18 +54,12 @@ class OverridableConfigRequirementTest extends \MediaWikiUnitTestCase {
'logged_in' => false,
'logged_out' => true,
],
// is A-B test enabled
false,
// note 0 = anon user
0,
// use central id lookup?
false,
// `languageinheader` query param
null,
// AB test name
'VectorLanguageInHeaderTreatmentABTest',
true,
'Anon users should get new treatment if enabled when A/B test disabled'
'Anon users should get new treatment if enabled'
],
[
// Is language enabled
@ -80,184 +67,11 @@ class OverridableConfigRequirementTest extends \MediaWikiUnitTestCase {
'logged_in' => true,
'logged_out' => false,
],
// is A-B test enabled
false,
2,
// use central id lookup?
false,
// `languageinheader` query param
null,
// AB test name
'VectorLanguageInHeaderTreatmentABTest',
true,
'Logged in users should get new treatment if enabled when A/B test disabled'
],
[
// Is language enabled
[
'logged_in' => true,
'logged_out' => false,
],
// is A-B test enabled
false,
1,
// use central id lookup?
false,
// `languageinheader` query param
null,
// AB test name
'VectorLanguageInHeaderTreatmentABTest',
true,
'All odd logged in users should get new treatent when A/B test disabled'
],
[
// Is language enabled
[
'logged_in' => false,
'logged_out' => true,
],
// is A-B test enabled
true,
// note 0 = anon user
0,
// use central id lookup?
false,
// `languageinheader` query param
null,
// AB test name
'VectorLanguageInHeaderTreatmentABTest',
true,
// Ab test is only for logged in users
'Anon users with a/b test enabled should see new treatment when config enabled'
],
[
// Is language enabled
[
'logged_in' => false,
'logged_out' => false,
],
// is A-B test enabled
true,
//
// note 0 = anon user
0,
// use central id lookup?
false,
// `languageinheader` query param
null,
// AB test name
'VectorLanguageInHeaderTreatmentABTest',
false,
// Ab test is only for logged in users
'Anon users with a/b test enabled should see old treatment when config disabled'
],
[
// Is language enabled
[
'logged_in' => true,
'logged_out' => false,
],
// is A-B test enabled
true,
2,
// use central id lookup?
false,
// `languageinheader` query param
null,
// AB test name
'VectorLanguageInHeaderTreatmentABTest',
true,
'Even logged in users get new treatment when A/B test enabled'
],
[
// Is language enabled
[
'logged_in' => true,
'logged_out' => false,
],
// is A-B test enabled
true,
1,
// use central id lookup?
false,
// `languageinheader` query param
null,
// AB test name
'VectorLanguageInHeaderTreatmentABTest',
false,
'Odd logged in users do not get new treatment when A/B test enabled'
],
[
// Is language enabled
[
'logged_in' => true,
'logged_out' => false,
],
// is A-B test enabled
true,
2,
// use central id lookup?
true,
// `languageinheader` query param
null,
// AB test name
'VectorLanguageInHeaderTreatmentABTest',
true,
'With CentralIdLookup, even logged in users get new treatment when A/B test enabled'
],
[
// Is language enabled
[
'logged_in' => true,
'logged_out' => false,
],
// is A-B test enabled
true,
1,
// use central id lookup?
true,
// `languageinheader` query param
null,
// AB test name
'VectorLanguageInHeaderTreatmentABTest',
false,
'With CentralIdLookup, odd logged in users do not get new treatment when A/B test enabled'
],
[
// Is language enabled
[
'logged_in' => true,
'logged_out' => false,
],
// is A-B test enabled
true,
1,
// use central id lookup?
false,
// `languageinheader` query param
"1",
// AB test name
'VectorLanguageInHeaderTreatmentABTest',
true,
'Odd logged in users get new treatment when A/B test enabled and query param set to "1"'
],
[
// Is language enabled
[
'logged_in' => true,
'logged_out' => false,
],
// is A-B test enabled
true,
1,
// use central id lookup?
false,
// `languageinheader` query param
"0",
// AB test name
'VectorLanguageInHeaderTreatmentABTest',
false,
'Even logged in users get old treatment when A/B test enabled and query param set to "0"'
'Even logged in users get old treatment when query param set to "0"'
],
[
// Is language enabled
@ -265,17 +79,11 @@ class OverridableConfigRequirementTest extends \MediaWikiUnitTestCase {
'logged_in' => false,
'logged_out' => false,
],
// is A-B test enabled
false,
1,
// use central id lookup?
false,
// `languageinheader` query param
"1",
// AB test name
null,
true,
'Users get new treatment when query param set to "1" regardless of state of A/B test or config flags'
'Users get new treatment when query param set to "1" regardless of state of config flags'
],
[
// Is language enabled
@ -283,17 +91,11 @@ class OverridableConfigRequirementTest extends \MediaWikiUnitTestCase {
'logged_in' => false,
'logged_out' => false,
],
// is A-B test enabled
false,
1,
// use central id lookup?
false,
// `languageinheader` query param
"0",
// AB test name
null,
false,
'Users get old treatment when query param set to "0" regardless of state of A/B test or config flags'
'Users get old treatment when query param set to "0" regardless of state of config flags'
],
];
}
@ -302,27 +104,20 @@ class OverridableConfigRequirementTest extends \MediaWikiUnitTestCase {
* @covers ::isMet
* @dataProvider providerLanguageInHeaderTreatmentRequirement
* @param bool $configValue
* @param bool $abValue
* @param int $userId
* @param bool $useCentralIdLookup
* @param string|null $queryParam
* @param string|null $testName
* @param bool $expected
* @param string $msg
*/
public function testLanguageInHeaderTreatmentRequirement(
$configValue,
$abValue,
$userId,
$useCentralIdLookup,
$queryParam,
$testName,
$expected,
$msg
) {
$config = new HashConfig( [
Constants::CONFIG_KEY_LANGUAGE_IN_HEADER => $configValue,
$testName => $abValue,
] );
$user = $this->createMock( User::class );
@ -333,18 +128,13 @@ class OverridableConfigRequirementTest extends \MediaWikiUnitTestCase {
$request->method( 'getCheck' )->willReturn( $queryParam !== null );
$request->method( 'getBool' )->willReturn( (bool)$queryParam );
$centralIdLookup = $this->createMock( CentralIdLookup::class );
$centralIdLookup->method( 'centralIdFromLocalUser' )->willReturn( $userId );
$requirement = new OverridableConfigRequirement(
$config,
$user,
$request,
$useCentralIdLookup ? $centralIdLookup : null,
'VectorLanguageInHeader',
'LanguageInHeader',
'languageinheader',
$testName ?? null
'languageinheader'
);
$this->assertSame( $expected, $requirement->isMet(), $msg );