diff --git a/includes/FeatureManagement/Requirements/OverridableConfigRequirement.php b/includes/FeatureManagement/Requirements/OverridableConfigRequirement.php index 3fe3a3b36..4de2049bf 100644 --- a/includes/FeatureManagement/Requirements/OverridableConfigRequirement.php +++ b/includes/FeatureManagement/Requirements/OverridableConfigRequirement.php @@ -75,11 +75,6 @@ final class OverridableConfigRequirement implements Requirement { */ private $user; - /** - * @var WebRequest - */ - private $request; - /** * @var string */ @@ -91,9 +86,9 @@ final class OverridableConfigRequirement implements Requirement { private $requirementName; /** - * @var string + * @var OverrideableRequirementHelper */ - private $overrideName; + private $helper; /** * This constructor accepts all dependencies needed to determine whether @@ -114,10 +109,9 @@ final class OverridableConfigRequirement implements Requirement { ) { $this->config = $config; $this->user = $user; - $this->request = $request; $this->configName = $configName; $this->requirementName = $requirementName; - $this->overrideName = strtolower( $configName ); + $this->helper = new OverrideableRequirementHelper( $request, $requirementName ); } /** @@ -135,12 +129,9 @@ final class OverridableConfigRequirement implements Requirement { * @inheritDoc */ public function isMet(): bool { - // Check query parameter. - if ( $this->request->getCheck( $this->overrideName ) ) { - return $this->request->getBool( $this->overrideName ); - } - if ( $this->request->getCheck( $this->configName ) ) { - return $this->request->getBool( $this->configName ); + $isMet = $this->helper->isMet(); + if ( $isMet !== null ) { + return $isMet; } // If AB test is not enabled, fallback to checking config state. diff --git a/includes/FeatureManagement/Requirements/OverrideableRequirementHelper.php b/includes/FeatureManagement/Requirements/OverrideableRequirementHelper.php new file mode 100644 index 000000000..a074bcf0a --- /dev/null +++ b/includes/FeatureManagement/Requirements/OverrideableRequirementHelper.php @@ -0,0 +1,84 @@ +request = $request; + $this->overrideName = 'vector' . strtolower( $requirementName ); + $this->requirementName = $requirementName; + } + + /** + * Check query parameter to override config or not. + * Then check for AB test value. + * Fallback to config value. + * + * @return bool|null + */ + public function isMet() { + // Check query parameter. + if ( $this->request->getCheck( $this->overrideName ) ) { + return $this->request->getBool( $this->overrideName ); + } + $vectorReq = 'Vector' . $this->requirementName; + if ( $this->request->getCheck( $vectorReq ) ) { + return $this->request->getBool( $vectorReq ); + } + return null; + } +} diff --git a/includes/FeatureManagement/Requirements/UserPreferenceRequirement.php b/includes/FeatureManagement/Requirements/UserPreferenceRequirement.php index 99a618cfa..7fcc083a0 100644 --- a/includes/FeatureManagement/Requirements/UserPreferenceRequirement.php +++ b/includes/FeatureManagement/Requirements/UserPreferenceRequirement.php @@ -25,6 +25,7 @@ use MediaWiki\Skins\Vector\FeatureManagement\Requirement; use MediaWiki\Title\Title; use MediaWiki\User\UserOptionsLookup; use User; +use WebRequest; /** * @package MediaWiki\Skins\Vector\FeatureManagement\Requirements @@ -56,6 +57,11 @@ final class UserPreferenceRequirement implements Requirement { */ private $title; + /** + * @var OverrideableRequirementHelper + */ + private $helper; + /** * This constructor accepts all dependencies needed to determine whether * the overridable config is enabled for the current user and request. @@ -64,6 +70,7 @@ final class UserPreferenceRequirement implements Requirement { * @param UserOptionsLookup $userOptionsLookup * @param string $optionName The name of the user preference. * @param string $requirementName The name of the requirement presented to FeatureManager. + * @param WebRequest $request * @param Title|null $title */ public function __construct( @@ -71,6 +78,7 @@ final class UserPreferenceRequirement implements Requirement { UserOptionsLookup $userOptionsLookup, string $optionName, string $requirementName, + WebRequest $request, $title = null ) { $this->user = $user; @@ -78,6 +86,7 @@ final class UserPreferenceRequirement implements Requirement { $this->optionName = $optionName; $this->requirementName = $requirementName; $this->title = $title; + $this->helper = new OverrideableRequirementHelper( $request, $requirementName ); } /** @@ -113,6 +122,7 @@ final class UserPreferenceRequirement implements Requirement { * @inheritDoc */ public function isMet(): bool { - return $this->isPreferenceEnabled(); + $override = $this->helper->isMet(); + return $override ?? $this->isPreferenceEnabled(); } } diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 57cb85796..b25ac94e2 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -50,6 +50,7 @@ return [ ); $context = RequestContext::getMain(); + $request = $context->getRequest(); // Feature: Languages in sidebar // ================================ @@ -57,7 +58,7 @@ return [ new OverridableConfigRequirement( $services->getMainConfig(), $context->getUser(), - $context->getRequest(), + $request, Constants::CONFIG_KEY_LANGUAGE_IN_HEADER, Constants::REQUIREMENT_LANGUAGE_IN_HEADER ) @@ -89,7 +90,7 @@ return [ new OverridableConfigRequirement( $config, $context->getUser(), - $context->getRequest(), + $request, Constants::CONFIG_KEY_LANGUAGE_IN_HEADER, $requirementName ) @@ -111,7 +112,7 @@ return [ new OverridableConfigRequirement( $services->getMainConfig(), $context->getUser(), - $context->getRequest(), + $request, Constants::CONFIG_LANGUAGE_IN_MAIN_PAGE_HEADER, Constants::REQUIREMENT_LANGUAGE_IN_MAIN_PAGE_HEADER ) @@ -138,7 +139,7 @@ return [ new OverridableConfigRequirement( $services->getMainConfig(), $context->getUser(), - $context->getRequest(), + $request, Constants::CONFIG_STICKY_HEADER, Constants::REQUIREMENT_STICKY_HEADER ) @@ -167,6 +168,7 @@ return [ $services->getUserOptionsLookup(), Constants::PREF_KEY_PAGE_TOOLS_PINNED, Constants::REQUIREMENT_PAGE_TOOLS_PINNED, + $request, $context->getTitle() ) ); @@ -188,6 +190,7 @@ return [ $services->getUserOptionsLookup(), Constants::PREF_KEY_TOC_PINNED, Constants::REQUIREMENT_TOC_PINNED, + $request, $context->getTitle() ) ); @@ -208,6 +211,7 @@ return [ $services->getUserOptionsLookup(), Constants::PREF_KEY_MAIN_MENU_PINNED, Constants::REQUIREMENT_MAIN_MENU_PINNED, + $request, $context->getTitle() ) ); @@ -229,6 +233,7 @@ return [ $services->getUserOptionsLookup(), Constants::PREF_KEY_LIMITED_WIDTH, Constants::REQUIREMENT_LIMITED_WIDTH, + $request, $context->getTitle() ) ); @@ -245,7 +250,7 @@ return [ $featureManager->registerRequirement( new LimitedWidthContentRequirement( $services->getMainConfig(), - $context->getRequest(), + $request, $context->getTitle() ) ); @@ -263,7 +268,7 @@ return [ new OverridableConfigRequirement( $services->getMainConfig(), $context->getUser(), - $context->getRequest(), + $request, Constants::CONFIG_ZEBRA_DESIGN, Constants::REQUIREMENT_ZEBRA_DESIGN ) @@ -285,6 +290,7 @@ return [ $services->getUserOptionsLookup(), Constants::PREF_KEY_FONT_SIZE, Constants::REQUIREMENT_FONT_SIZE, + $request, $context->getTitle() ) ); @@ -304,7 +310,7 @@ return [ new OverridableConfigRequirement( $services->getMainConfig(), $context->getUser(), - $context->getRequest(), + $request, Constants::CONFIG_KEY_CLIENT_PREFERENCES, Constants::REQUIREMENT_CLIENT_PREFERENCES ) @@ -326,6 +332,7 @@ return [ $services->getUserOptionsLookup(), Constants::PREF_KEY_TYPOGRAPHY_SURVEY, Constants::REQUIREMENT_TYPOGRAPHY_SURVEY, + $request, $context->getTitle() ) ); diff --git a/tests/phpunit/unit/FeatureManagement/Requirements/OverridableConfigRequirementTest.php b/tests/phpunit/unit/FeatureManagement/Requirements/OverridableConfigRequirementTest.php index 43e2f4f78..3e8e90b60 100644 --- a/tests/phpunit/unit/FeatureManagement/Requirements/OverridableConfigRequirementTest.php +++ b/tests/phpunit/unit/FeatureManagement/Requirements/OverridableConfigRequirementTest.php @@ -133,8 +133,7 @@ class OverridableConfigRequirementTest extends \MediaWikiUnitTestCase { $user, $request, 'VectorLanguageInHeader', - 'LanguageInHeader', - 'languageinheader' + 'LanguageInHeader' ); $this->assertSame( $expected, $requirement->isMet(), $msg ); diff --git a/tests/phpunit/unit/FeatureManagement/Requirements/UserPreferenceRequirementTest.php b/tests/phpunit/unit/FeatureManagement/Requirements/UserPreferenceRequirementTest.php index 3d08faad8..2bb2a62f5 100644 --- a/tests/phpunit/unit/FeatureManagement/Requirements/UserPreferenceRequirementTest.php +++ b/tests/phpunit/unit/FeatureManagement/Requirements/UserPreferenceRequirementTest.php @@ -2,6 +2,7 @@ namespace MediaWiki\Skins\Vector\Tests\Unit\FeatureManagement\Requirements; +use FauxRequest; use MediaWiki\Skins\Vector\FeatureManagement\Requirements\UserPreferenceRequirement; use MediaWiki\Title\Title; use MediaWiki\User\UserOptionsLookup; @@ -85,6 +86,7 @@ final class UserPreferenceRequirementTest extends \MediaWikiUnitTestCase { ) { $user = $this->createMock( User::class ); $title = $isTitlePresent ? $this->createMock( Title::class ) : null; + $request = new FauxRequest(); $userOptionsLookup = $this->createMock( UserOptionsLookup::class ); $userOptionsLookup->method( 'getOption' )->willReturn( $isEnabled ); @@ -94,6 +96,7 @@ final class UserPreferenceRequirementTest extends \MediaWikiUnitTestCase { $userOptionsLookup, 'userOption', 'userRequirement', + $request, $title );