jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/336945 )
Change subject: Hygiene: Rename isEnabledByUser to shouldSendModuleToUser
......................................................................
Hygiene: Rename isEnabledByUser to shouldSendModuleToUser
Sometimes we make choices on the users behalf if we don't have
sufficient information, so this name is more applicable.
If beta features is disabled and the user is anon they have not
explicitly opted in.
Change-Id: I5d816f569fc54f8bf74d6e5a06246b7fa7036e06
---
M Popups.hooks.php
M includes/PopupsContext.php
M resources/ext.popups/isEnabled.js
M tests/phpunit/PopupsContextTest.php
M tests/phpunit/PopupsHooksTest.php
M tests/qunit/ext.popups/isEnabled.test.js
6 files changed, 26 insertions(+), 26 deletions(-)
Approvals:
Bmansurov: Looks good to me, approved
jenkins-bot: Verified
diff --git a/Popups.hooks.php b/Popups.hooks.php
index 9927744..de44c5f 100644
--- a/Popups.hooks.php
+++ b/Popups.hooks.php
@@ -95,7 +95,7 @@
return true;
}
- if ( !$module->isBetaFeatureEnabled() ||
$module->isEnabledByUser( $user ) ) {
+ if ( !$module->isBetaFeatureEnabled() ||
$module->shouldSendModuleToUser( $user ) ) {
$out->addModules( [ 'ext.popups' ] );
}
@@ -153,8 +153,8 @@
* MakeGlobalVariablesScript hook handler.
*
* Variables added:
- * * `wgPopupsIsEnabledByUser' - The server's notion of whether or not
the
- * user has enabled Page Previews (see
`\Popups\PopupsContext#isEnabledByUser`).
+ * * `wgPopupsShouldSendModuleToUser' - The server's notion of whether
or not the
+ * user has enabled Page Previews (see
`\Popups\PopupsContext#shouldSendModuleToUser`).
* * `wgPopupsConflictsWithNavPopupGadget' - The server's notion of
whether or not the
* user has enabled conflicting Navigational Popups Gadget.
*
@@ -165,7 +165,7 @@
$module = PopupsContext::getInstance();
$user = $out->getUser();
- $vars['wgPopupsIsEnabledByUser'] = $module->isEnabledByUser(
$user );
+ $vars['wgPopupsShouldSendModuleToUser'] =
$module->shouldSendModuleToUser( $user );
$vars['wgPopupsConflictsWithNavPopupGadget'] =
$module->conflictsWithNavPopupsGadget(
$user );
}
diff --git a/includes/PopupsContext.php b/includes/PopupsContext.php
index bd0864f..6491759 100644
--- a/includes/PopupsContext.php
+++ b/includes/PopupsContext.php
@@ -147,7 +147,7 @@
* @param \User $user
* @return bool
*/
- public function isEnabledByUser( \User $user ) {
+ public function shouldSendModuleToUser( \User $user ) {
if ( $this->isBetaFeatureEnabled() ) {
return $user->isAnon() ? false :
\BetaFeatures::isFeatureEnabled( $user,
self::PREVIEWS_BETA_PREFERENCE_NAME );
diff --git a/resources/ext.popups/isEnabled.js
b/resources/ext.popups/isEnabled.js
index cf627db..2649b4f 100644
--- a/resources/ext.popups/isEnabled.js
+++ b/resources/ext.popups/isEnabled.js
@@ -21,7 +21,7 @@
*/
mw.popups.isEnabled = function ( user, userSettings, config ) {
if ( !user.isAnon() ) {
- return config.get( 'wgPopupsIsEnabledByUser' );
+ return config.get( 'wgPopupsShouldSendModuleToUser' );
}
if ( config.get( 'wgPopupsBetaFeature' ) ) {
diff --git a/tests/phpunit/PopupsContextTest.php
b/tests/phpunit/PopupsContextTest.php
index e5956b3..b7b819c 100644
--- a/tests/phpunit/PopupsContextTest.php
+++ b/tests/phpunit/PopupsContextTest.php
@@ -102,26 +102,26 @@
}
/**
- * @covers ::isEnabledByUser
+ * @covers ::shouldSendModuleToUser
* @covers ::isBetaFeatureEnabled
- * @dataProvider provideTestDataForIsEnabledByUser
+ * @dataProvider provideTestDataForShouldSendModuleToUser
* @param bool $optIn
* @param bool $expected
*/
- public function testIsEnabledByUser( $optIn, $expected ) {
+ public function testShouldSendModuleToUser( $optIn, $expected ) {
$this->setMwGlobals( [
"wgPopupsBetaFeature" => false
] );
$context = PopupsContext::getInstance();
$user = $this->getMutableTestUser()->getUser();
$user->setOption(
PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME, $optIn );
- $this->assertEquals( $context->isEnabledByUser( $user ),
$expected );
+ $this->assertEquals( $context->shouldSendModuleToUser( $user ),
$expected );
}
/**
* @return array/
*/
- public function provideTestDataForIsEnabledByUser() {
+ public function provideTestDataForShouldSendModuleToUser() {
return [
[
"optin" => PopupsContext::PREVIEWS_ENABLED,
@@ -135,13 +135,13 @@
}
/**
- * @covers ::isEnabledByUser
+ * @covers ::shouldSendModuleToUser
* @covers ::isBetaFeatureEnabled
- * @dataProvider provideTestDataForIsEnabledByUserWhenBetaEnabled
+ * @dataProvider provideTestDataForShouldSendModuleToUserWhenBetaEnabled
* @param bool $optIn
* @param bool $expected
*/
- public function testIsEnabledByUserWhenBetaEnabled( $optIn, $expected )
{
+ public function testShouldSendModuleToUserWhenBetaEnabled( $optIn,
$expected ) {
if ( !class_exists( 'BetaFeatures' ) ) {
$this->markTestSkipped( 'Skipped as BetaFeatures is not
available' );
}
@@ -151,12 +151,12 @@
$context = PopupsContext::getInstance();
$user = $this->getMutableTestUser()->getUser();
$user->setOption( PopupsContext::PREVIEWS_BETA_PREFERENCE_NAME,
$optIn );
- $this->assertEquals( $context->isEnabledByUser( $user ),
$expected );
+ $this->assertEquals( $context->shouldSendModuleToUser( $user ),
$expected );
}
/**
* Check that Page Previews are disabled for anonymous user
- * @covers ::isEnabledByUser
+ * @covers ::shouldSendModuleToUser
* @covers ::isBetaFeatureEnabled
* @dataProvider providerAnonUserHasDisabledPagePreviews
*/
@@ -170,7 +170,7 @@
] );
$context = PopupsContext::getInstance();
- $this->assertEquals( $expected, $context->isEnabledByUser(
$user ) );
+ $this->assertEquals( $expected,
$context->shouldSendModuleToUser( $user ) );
}
public static function providerAnonUserHasDisabledPagePreviews() {
@@ -184,7 +184,7 @@
/**
* @return array/
*/
- public function provideTestDataForIsEnabledByUserWhenBetaEnabled() {
+ public function
provideTestDataForShouldSendModuleToUserWhenBetaEnabled() {
return [
[
"optin" => PopupsContext::PREVIEWS_ENABLED,
diff --git a/tests/phpunit/PopupsHooksTest.php
b/tests/phpunit/PopupsHooksTest.php
index 488ff26..d381a51 100644
--- a/tests/phpunit/PopupsHooksTest.php
+++ b/tests/phpunit/PopupsHooksTest.php
@@ -274,7 +274,7 @@
* @covers ::onBeforePageDisplay
* @dataProvider providerOnBeforePageDisplay
*/
- public function testOnBeforePageDisplay( $isEnabledByUser,
$isBetaFeatureEnabled, $isCodeLoaded ) {
+ public function testOnBeforePageDisplay( $shouldSendModuleToUser,
$isBetaFeatureEnabled, $isCodeLoaded ) {
$skinMock = $this->getMock( Skin::class );
$outPageMock = $this->getMock(
@@ -290,7 +290,7 @@
->with( [ 'ext.popups' ] );
$contextMock = $this->getMockBuilder(
PopupsContextTestWrapper::class )
- ->setMethods( [ 'areDependenciesMet',
'isBetaFeatureEnabled', 'isEnabledByUser' ] )
+ ->setMethods( [ 'areDependenciesMet',
'isBetaFeatureEnabled', 'shouldSendModuleToUser' ] )
->disableOriginalConstructor()
->getMock();
@@ -303,8 +303,8 @@
->will( $this->returnValue( $isBetaFeatureEnabled ) );
$contextMock->expects( $this->any() )
- ->method( 'isEnabledByUser' )
- ->will( $this->returnValue( $isEnabledByUser ) );
+ ->method( 'shouldSendModuleToUser' )
+ ->will( $this->returnValue( $shouldSendModuleToUser ) );
PopupsContextTestWrapper::injectTestInstance( $contextMock );
PopupsHooks::onBeforePageDisplay( $outPageMock, $skinMock );
@@ -325,11 +325,11 @@
->willReturn( $user );
$contextMock = $this->getMockBuilder(
PopupsContextTestWrapper::class )
- ->setMethods( [ 'isEnabledByUser',
'conflictsWithNavPopupsGadget' ] )
+ ->setMethods( [ 'shouldSendModuleToUser',
'conflictsWithNavPopupsGadget' ] )
->disableOriginalConstructor()
->getMock();
$contextMock->expects( $this->once() )
- ->method( 'isEnabledByUser' )
+ ->method( 'shouldSendModuleToUser' )
->with( $user )
->willReturn( false );
$contextMock->expects( $this->once() )
@@ -344,7 +344,7 @@
PopupsHooks::onMakeGlobalVariablesScript( $vars, $outputPage );
$this->assertCount( 2, $vars );
- $this->assertFalse( $vars[ 'wgPopupsIsEnabledByUser' ] );
+ $this->assertFalse( $vars[ 'wgPopupsShouldSendModuleToUser' ] );
$this->assertFalse( $vars[
'wgPopupsConflictsWithNavPopupGadget' ] );
}
}
diff --git a/tests/qunit/ext.popups/isEnabled.test.js
b/tests/qunit/ext.popups/isEnabled.test.js
index 5eb1e10..c4ba9b8 100644
--- a/tests/qunit/ext.popups/isEnabled.test.js
+++ b/tests/qunit/ext.popups/isEnabled.test.js
@@ -54,7 +54,7 @@
userSettings = createStubUserSettings( false ),
config = new mw.Map();
- config.set( 'wgPopupsIsEnabledByUser', true );
+ config.set( 'wgPopupsShouldSendModuleToUser', true );
assert.ok(
mw.popups.isEnabled( user, userSettings, config ),
--
To view, visit https://gerrit.wikimedia.org/r/336945
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I5d816f569fc54f8bf74d6e5a06246b7fa7036e06
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: mpga
Gerrit-Owner: Jdlrobson <[email protected]>
Gerrit-Reviewer: Bmansurov <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
