From 959bf40d9bdb114ea5a27544e1e17b66b215cd7b Mon Sep 17 00:00:00 2001 From: Piotr Miazga Date: Fri, 16 Dec 2016 03:47:52 +0100 Subject: [PATCH] Implement necessary wiring for preferences Changes: - introduce unit tests - don't send module for anonymous users - renamed Module to Context - remove Config dependency from Popups.hooks.php Bug: T146889 Change-Id: I3cbcdc1303411b28b613afa6dd1a00b410891471 --- Popups.hooks.php | 98 ++++++++++------- extension.json | 12 +-- includes/Module.php | 35 ------ includes/PopupsContext.php | 124 +++++++++++++++++++++ tests/phpunit/ModuleTest.php | 49 --------- tests/phpunit/PopupsContextTest.php | 161 ++++++++++++++++++++++++++++ tests/phpunit/PopupsHooksTest.php | 117 ++++++++++++++++++++ 7 files changed, 467 insertions(+), 129 deletions(-) delete mode 100644 includes/Module.php create mode 100644 includes/PopupsContext.php delete mode 100644 tests/phpunit/ModuleTest.php create mode 100644 tests/phpunit/PopupsContextTest.php create mode 100644 tests/phpunit/PopupsHooksTest.php diff --git a/Popups.hooks.php b/Popups.hooks.php index 0f29874ba..042ab0433 100644 --- a/Popups.hooks.php +++ b/Popups.hooks.php @@ -18,20 +18,19 @@ * @file * @ingroup extensions */ -use MediaWiki\Logger\LoggerFactory; +use Popups\PopupsContext; class PopupsHooks { - const PREVIEWS_ENABLED = 'enabled'; - const PREVIEWS_DISABLED = 'disabled'; - const PREVIEWS_OPTIN_PREFERENCE_NAME = 'popups-enable'; const PREVIEWS_PREFERENCES_SECTION = 'rendering/reading'; - static function getPreferences( User $user, array &$prefs ) { + private static $context; + + static function onGetBetaPreferences( User $user, array &$prefs ) { global $wgExtensionAssetsPath; - if ( self::getConfig()->get( 'PopupsBetaFeature' ) !== true ) { + if ( self::getModuleContext()->getConfig()->get( 'PopupsBetaFeature' ) !== true ) { return; } - $prefs['popups'] = [ + $prefs[PopupsContext::PREVIEWS_BETA_PREFERENCE_NAME] = [ 'label-message' => 'popups-message', 'desc-message' => 'popups-desc', 'screenshot' => [ @@ -53,60 +52,52 @@ class PopupsHooks { * @param array $prefs */ static function onGetPreferences( User $user, array &$prefs ) { - $module = new \Popups\Module( self::getConfig() ); + $module = self::getModuleContext(); if ( !$module->showPreviewsOptInOnPreferencesPage() ) { return; } - $prefs[self::PREVIEWS_OPTIN_PREFERENCE_NAME] = [ + $prefs[PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME] = [ 'type' => 'radio', 'label-message' => 'popups-prefs-optin-title', 'options' => [ - wfMessage( 'popups-prefs-optin-enabled-label' )->text() => self::PREVIEWS_ENABLED, - wfMessage( 'popups-prefs-optin-disabled-label' )->text() => self::PREVIEWS_DISABLED + wfMessage( 'popups-prefs-optin-enabled-label' )->text() + => PopupsContext::PREVIEWS_ENABLED, + wfMessage( 'popups-prefs-optin-disabled-label' )->text() + => PopupsContext::PREVIEWS_DISABLED ], 'section' => self::PREVIEWS_PREFERENCES_SECTION ]; } /** - * @return Config + * @return PopupsContext */ - public static function getConfig() { - static $config; - if ( !$config ) { - $config = ConfigFactory::getDefaultInstance()->makeConfig( 'popups' ); + private static function getModuleContext() { + + if ( !self::$context ) { + self::$context = new \Popups\PopupsContext(); } - return $config; + return self::$context; + } + + private static function areDependenciesMet() { + $registry = ExtensionRegistry::getInstance(); + return $registry->isLoaded( 'TextExtracts' ) && class_exists( 'ApiQueryPageImages' ); } public static function onBeforePageDisplay( OutputPage &$out, Skin &$skin ) { - // Enable only if the user has turned it on in Beta Preferences, or BetaFeatures is not installed. - // Will only be loaded if PageImages & TextExtracts extensions are installed. + $module = self::getModuleContext(); - $registry = ExtensionRegistry::getInstance(); - if ( !$registry->isLoaded( 'TextExtracts' ) || !class_exists( 'ApiQueryPageImages' ) ) { - $logger = LoggerFactory::getInstance( 'popups' ); + if ( !self::areDependenciesMet() ) { + $logger = $module->getLogger(); $logger->error( 'Popups requires the PageImages and TextExtracts extensions.' ); return true; } - $config = self::getConfig(); - - if ( $config->get( 'PopupsBetaFeature' ) === true ) { - if ( !class_exists( 'BetaFeatures' ) ) { - $logger = LoggerFactory::getInstance( 'popups' ); - $logger->error( 'PopupsMode cannot be used as a beta feature unless ' . - 'the BetaFeatures extension is present.' ); - return true; - } - if ( !BetaFeatures::isFeatureEnabled( $skin->getUser(), 'popups' ) ) { - return true; - } + if ( $module->isEnabledByUser( $skin->getUser() ) ) { + $out->addModules( [ 'ext.popups' ] ); } - - $out->addModules( [ 'ext.popups' ] ); - return true; } @@ -152,7 +143,8 @@ class PopupsHooks { * @param array $vars */ public static function onResourceLoaderGetConfigVars( array &$vars ) { - $conf = self::getConfig(); + $module = self::getModuleContext(); + $conf = $module->getConfig(); $vars['wgPopupsSchemaPopupsSamplingRate'] = $conf->get( 'SchemaPopupsSamplingRate' ); } @@ -169,7 +161,35 @@ class PopupsHooks { * or when ConfigRegistry gets populated before calling `callback` ExtensionRegistry hook */ $config = \MediaWiki\MediaWikiServices::getInstance()->getMainConfig(); - $wgDefaultUserOptions[ self::PREVIEWS_OPTIN_PREFERENCE_NAME ] = + $wgDefaultUserOptions[ PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME ] = $config->get( 'PopupsOptInDefaultState' ); } + + /** + * Inject Mocked context + * As there is no service registration this is used for tests only. + * + * @param PopupsContext $context + * @throws MWException + */ + public static function injectContext( PopupsContext $context ) { + if ( !defined( 'MW_PHPUNIT_TEST' ) ) { + throw new MWException( 'injectContext() must not be used outside unit tests.' ); + } + self::$context = $context; + } + + /** + * Remove cached context. + * As there is no service registration this is used for tests only. + * + * + * @throws MWException + */ + public static function resetContext() { + if ( !defined( 'MW_PHPUNIT_TEST' ) ) { + throw new MWException( 'injectContext() must not be used outside unit tests.' ); + } + self::$context = null; + } } diff --git a/extension.json b/extension.json index 0168062ed..200b08f55 100644 --- a/extension.json +++ b/extension.json @@ -10,14 +10,14 @@ "type": "betafeatures", "AutoloadClasses": { "PopupsHooks": "Popups.hooks.php", - "Popups\\Module": "includes/Module.php" + "Popups\\PopupsContext": "includes/PopupsContext.php" }, "ConfigRegistry": { "popups": "GlobalVarConfig::newInstance" }, "Hooks": { "GetBetaFeaturePreferences": [ - "PopupsHooks::getPreferences" + "PopupsHooks::onGetBetaPreferences" ], "BeforePageDisplay": [ "PopupsHooks::onBeforePageDisplay" @@ -46,10 +46,10 @@ "PopupsBetaFeature": false, "@SchemaPopupsSamplingRate": "@var number: Sample rate for logging events to Schema:Popups.", "SchemaPopupsSamplingRate": 0, - "@PopupsHideOptInOnPreferencesPage": "@var bool: Whether the option to enable/disable Page Previews should be hidden on Preferences page. Please note if PopupsBetaFeature is set to true this option will be always hidden. False by default", - "PopupsHideOptInOnPreferencesPage": true, - "@PopupsOptInDefaultState" : "@var string:[enabled|disabled] Default Page Previews visibility", - "PopupsOptInDefaultState" : "disabled" + "@PopupsHideOptInOnPreferencesPage": "@var bool: Whether the option to senable/disable Page Previews should be hidden on Preferences page. Please note if PopupsBetaFeature is set to true this option will be always hidden. False by default", + "PopupsHideOptInOnPreferencesPage": false, + "@PopupsOptInDefaultState" : "@var string:['1'|'0'] Default Page Previews visibility. Has to be a string as a compatibility with beta feature settings", + "PopupsOptInDefaultState" : "0" }, "ResourceModules": { "ext.popups.images": { diff --git a/includes/Module.php b/includes/Module.php deleted file mode 100644 index 448aa1d09..000000000 --- a/includes/Module.php +++ /dev/null @@ -1,35 +0,0 @@ -config = $config; - } - - /** - * Are Page previews visible on User Preferences Page - * - * return @bool - */ - public function showPreviewsOptInOnPreferencesPage() { - return $this->config->get( 'PopupsBetaFeature' ) === false - && $this->config->get( 'PopupsHideOptInOnPreferencesPage' ) === false; - } -} diff --git a/includes/PopupsContext.php b/includes/PopupsContext.php new file mode 100644 index 000000000..8f77c8dfc --- /dev/null +++ b/includes/PopupsContext.php @@ -0,0 +1,124 @@ +. + * + * @file + * @ingroup extensions + */ +namespace Popups; + +use MediaWiki\Logger\LoggerFactory; + +/** + * Popups Module + * + * @package Popups + */ +class PopupsContext { + + /** + * Extension name + * @var string + */ + const EXTENSION_NAME = 'popups'; + /** + * Logger channel (name) + * @var string + */ + const LOGGER_CHANNEL = 'popups'; + /** + * User preference value for enabled Page Previews + * @var string + */ + const PREVIEWS_ENABLED = \HTMLFeatureField::OPTION_ENABLED; + /** + * User preference value for disabled Page Previews + * @var string + */ + const PREVIEWS_DISABLED = \HTMLFeatureField::OPTION_DISABLED; + /** + * User preference to enable/disable Page Previews + * Currently for BETA and regular opt in we use same preference name + * + * @var string + */ + const PREVIEWS_OPTIN_PREFERENCE_NAME = 'popups'; + /** + * User preference to enable/disable Page Preivews as a beta feature + * @var string + */ + const PREVIEWS_BETA_PREFERENCE_NAME = 'popups'; + + /** + * @var \Config + */ + private $config; + + /** + * Module constructor. + */ + public function __construct() { + $this->config = \MediaWiki\MediaWikiServices::getInstance() + ->getConfigFactory()->makeConfig( PopupsContext::EXTENSION_NAME ); + } + + /** + * Are Page previews visible on User Preferences Page + * + * return @bool + */ + public function showPreviewsOptInOnPreferencesPage() { + return $this->config->get( 'PopupsBetaFeature' ) === false + && $this->config->get( 'PopupsHideOptInOnPreferencesPage' ) === false; + } + + /** + * @param \User $user + * @return bool + */ + public function isEnabledByUser( \User $user ) { + if ( $user->isAnon() ) { + return false; + } + if ( $this->config->get( 'PopupsBetaFeature' ) ) { + if ( !class_exists( 'BetaFeatures' ) ) { + $this->getLogger()->error( 'PopupsMode cannot be used as a beta feature unless ' . + 'the BetaFeatures extension is present.' ); + return false; + } + return \BetaFeatures::isFeatureEnabled( $user, self::PREVIEWS_BETA_PREFERENCE_NAME ); + }; + return $user->getOption( self::PREVIEWS_OPTIN_PREFERENCE_NAME ) === self::PREVIEWS_ENABLED; + } + + /** + * Get module logger + * + * @return \Psr\Log\LoggerInterface + */ + public function getLogger() { + return LoggerFactory::getInstance( self::LOGGER_CHANNEL ); + } + + /** + * Get Module config + * + * @return \Config + */ + public function getConfig() { + return $this->config; + } +} diff --git a/tests/phpunit/ModuleTest.php b/tests/phpunit/ModuleTest.php deleted file mode 100644 index 1ea00079f..000000000 --- a/tests/phpunit/ModuleTest.php +++ /dev/null @@ -1,49 +0,0 @@ -assertEquals( $enabled, $module->showPreviewsOptInOnPreferencesPage() ); - } - - /** - * @return array - */ - public function provideConfigForShowPreviewsInOptIn() { - return [ - [ - "options" => [ - "PopupsBetaFeature" => false, - "PopupsHideOptInOnPreferencesPage" => false - ], - "enabled" => true - ], - [ - "options" => [ - "PopupsBetaFeature" => true, - "PopupsHideOptInOnPreferencesPage" => false - ], - "enabled" => false - ], - [ - "options" => [ - "PopupsBetaFeature" => false, - "PopupsHideOptInOnPreferencesPage" => true - ], - "enabled" => false - ] - ]; - } -} diff --git a/tests/phpunit/PopupsContextTest.php b/tests/phpunit/PopupsContextTest.php new file mode 100644 index 000000000..b584414ee --- /dev/null +++ b/tests/phpunit/PopupsContextTest.php @@ -0,0 +1,161 @@ +setMwGlobals( $config ); + $module = new Popups\PopupsContext(); + $this->assertEquals( $expected, $module->showPreviewsOptInOnPreferencesPage() ); + } + + /** + * @covers Popups\PopupsContext::__construct + * @covers Popups\PopupsContext::getConfig + */ + public function testContextAndConfigInitialization() { + $configMock = $this->getMock( Config::class ); + + $configFactoryMock = $this->getMock( ConfigFactory::class, [ 'makeConfig' ] ); + $configFactoryMock->expects( $this->once() ) + ->method( 'makeConfig' ) + ->with( PopupsContext::EXTENSION_NAME ) + ->will( $this->returnValue( $configMock ) ); + + $mwServices = $this->overrideMwServices(); + $mwServices->redefineService( 'ConfigFactory', function() use ( $configFactoryMock ) { + return $configFactoryMock; + } ); + + $module = new Popups\PopupsContext(); + $this->assertSame( $module->getConfig(), $configMock ); + } + + /** + * @return array + */ + public function provideConfigForShowPreviewsInOptIn() { + return [ + [ + "options" => [ + "wgPopupsBetaFeature" => false, + "wgPopupsHideOptInOnPreferencesPage" => false + ], + "expected" => true + ], + [ + "options" => [ + "wgPopupsBetaFeature" => true, + "wgPopupsHideOptInOnPreferencesPage" => false + ], + "expected" => false + ], + [ + "options" => [ + "wgPopupsBetaFeature" => false, + "wgPopupsHideOptInOnPreferencesPage" => true + ], + "expected" => false + ] + ]; + } + + /** + * @covers Popups\PopupsContext::isEnabledByUser + * @dataProvider provideTestDataForIsEnabledByUser + */ + public function testIsEnabledByUser( $optIn, $expected ) { + $this->setMwGlobals( [ + "wgPopupsBetaFeature" => false + ] ); + $module = new PopupsContext(); + $user = $this->getMutableTestUser()->getUser(); + $user->setOption( PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME, $optIn ); + $this->assertEquals( $module->isEnabledByUser( $user ), $expected ); + } + + /** + * @return array/ + */ + public function provideTestDataForIsEnabledByUser() { + return [ + [ + "optin" => PopupsContext::PREVIEWS_ENABLED, + 'expected' => true + ], + [ + "optin" => PopupsContext::PREVIEWS_DISABLED, + 'expected' => false + ] + ]; + } + + /** + * @covers Popups\PopupsContext::isEnabledByUser + * @dataProvider provideTestDataForIsEnabledByUserWhenBetaEnabled + */ + public function testIsEnabledByUserWhenBetaEnabled( $optIn, $expected ) { + if ( !class_exists( 'BetaFeatures' ) ) { + $this->markTestSkipped( 'Skipped as BetaFeatures is not available' ); + } + $this->setMwGlobals( [ + "wgPopupsBetaFeature" => true + ] ); + $module = new PopupsContext(); + $user = $this->getMutableTestUser()->getUser(); + $user->setOption( PopupsContext::PREVIEWS_BETA_PREFERENCE_NAME, $optIn ); + $this->assertEquals( $module->isEnabledByUser( $user ), $expected ); + } + + /** + * Check that Page Previews are disabled for anonymous user + */ + public function testAnonUserHasDisabledPagePreviews() { + $user = $this->getMutableTestUser()->getUser(); + $user->setId( 0 ); + $user->setOption( PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME, + PopupsContext::PREVIEWS_ENABLED ); + $this->setMwGlobals( [ + "wgPopupsBetaFeature" => false + ] ); + + $context = new PopupsContext(); + $this->assertEquals( false, $context->isEnabledByUser( $user ) ); + } + /** + * @return array/ + */ + public function provideTestDataForIsEnabledByUserWhenBetaEnabled() { + return [ + [ + "optin" => PopupsContext::PREVIEWS_ENABLED, + 'expected' => true + ], + [ + "optin" => PopupsContext::PREVIEWS_DISABLED, + 'expected' => false + ] + ]; + } + + /** + * @covers Popups\PopupsContext::getLogger + */ + public function testGetLogger() { + $loggerMock = $this->getMock( \Psr\Log\LoggerInterface::class ); + + $this->setLogger( PopupsContext::LOGGER_CHANNEL, $loggerMock ); + $context = new PopupsContext(); + $this->assertSame( $loggerMock, $context->getLogger() ); + } + +} diff --git a/tests/phpunit/PopupsHooksTest.php b/tests/phpunit/PopupsHooksTest.php new file mode 100644 index 000000000..f6108ac35 --- /dev/null +++ b/tests/phpunit/PopupsHooksTest.php @@ -0,0 +1,117 @@ + 'notEmpty' ]; + $this->setMwGlobals( [ 'wgPopupsBetaFeature' => false ] ); + + PopupsHooks::onGetBetaPreferences( $this->getTestUser()->getUser(), $prefs ); + $this->assertCount( 1, $prefs ); + $this->assertEquals( 'notEmpty', $prefs[ 'someNotEmptyValue'] ); + } + + /** + * @covers PopupsHooks::onGetBetaPreferences + */ + public function testOnGetBetaPreferencesBetaEnabled() { + $prefs = [ 'someNotEmptyValue' => 'notEmpty' ]; + $this->setMwGlobals( [ 'wgPopupsBetaFeature' => true ] ); + + PopupsHooks::onGetBetaPreferences( $this->getTestUser()->getUser(), $prefs ); + $this->assertCount( 2, $prefs ); + $this->assertArrayHasKey( \Popups\PopupsContext::PREVIEWS_BETA_PREFERENCE_NAME, $prefs ); + } + + /** + * @covers PopupsHooks::onGetPreferences + * @covers PopupsHooks::injectContext + */ + public function testOnGetPreferencesPreviewsDisabled() { + $contextMock = $this->getMock( \Popups\PopupsContext::class, + [ 'showPreviewsOptInOnPreferencesPage' ] ); + $contextMock->expects( $this->once() ) + ->method( 'showPreviewsOptInOnPreferencesPage' ) + ->will( $this->returnValue( false ) ); + + PopupsHooks::injectContext( $contextMock ); + $prefs = [ 'someNotEmptyValue' => 'notEmpty' ]; + + PopupsHooks::onGetPreferences( $this->getTestUser()->getUser(), $prefs ); + $this->assertCount( 1, $prefs ); + $this->assertEquals( 'notEmpty', $prefs[ 'someNotEmptyValue'] ); + } + + /** + * @covers PopupsHooks::onGetPreferences + * @covers PopupsHooks::injectContext + */ + public function testOnGetPreferencesPreviewsEnabled() { + $contextMock = $this->getMock( \Popups\PopupsContext::class, + [ 'showPreviewsOptInOnPreferencesPage' ] ); + $contextMock->expects( $this->once() ) + ->method( 'showPreviewsOptInOnPreferencesPage' ) + ->will( $this->returnValue( true ) ); + + PopupsHooks::injectContext( $contextMock ); + $prefs = [ 'someNotEmptyValue' => 'notEmpty' ]; + + PopupsHooks::onGetPreferences( $this->getTestUser()->getUser(), $prefs ); + $this->assertCount( 2, $prefs ); + $this->assertEquals( 'notEmpty', $prefs[ 'someNotEmptyValue'] ); + $this->assertArrayHasKey( \Popups\PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME, $prefs ); + } + + /** + * @covers PopupsHooks::onResourceLoaderTestModules + */ + public function testOnResourceLoaderTestModules() { + $testModules = [ 'someNotEmptyValue' => 'notEmpty' ]; + $resourceLoaderMock = $this->getMock( ResourceLoader::class ); + PopupsHooks::onResourceLoaderTestModules( $testModules, $resourceLoaderMock ); + + $this->assertCount( 2, $testModules ); + $this->assertEquals( 'notEmpty', $testModules[ 'someNotEmptyValue' ] ); + $this->assertArrayHasKey( 'qunit', $testModules, 'ResourceLoader expects qunit test modules' ); + $this->assertCount( 2, $testModules[ 'qunit' ], 'ResourceLoader expects 2 test modules. ' ); + } + + /** + * @covers PopupsHooks::onResourceLoaderGetConfigVars + */ + public function testOnResourceLoaderGetConfigVars() { + $vars = [ 'something' => 'notEmpty' ]; + $value = 10; + $this->setMwGlobals( [ 'wgSchemaPopupsSamplingRate' => $value ] ); + PopupsHooks::onResourceLoaderGetConfigVars( $vars ); + $this->assertCount( 2, $vars ); + $this->assertEquals( $value, $vars[ 'wgPopupsSchemaPopupsSamplingRate' ] ); + } + + /** + * @covers PopupsHooks::onExtensionRegistration + */ + public function testOnExtensionRegistration() { + global $wgDefaultUserOptions; + + $test = 'testValue'; + $this->setMwGlobals( [ 'wgPopupsOptInDefaultState' => $test ] ); + PopupsHooks::onExtensionRegistration(); + $this->assertEquals( $test, + $wgDefaultUserOptions[ \Popups\PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME ] ); + } + +}