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
This commit is contained in:
jdlrobson 2017-02-09 16:27:05 -08:00
parent 4e2ce1ce39
commit 64e7bfd620
6 changed files with 26 additions and 26 deletions

View file

@ -95,7 +95,7 @@ class PopupsHooks {
return true;
}
if ( !$module->isBetaFeatureEnabled() || $module->isEnabledByUser( $user ) ) {
if ( !$module->isBetaFeatureEnabled() || $module->shouldSendModuleToUser( $user ) ) {
$out->addModules( [ 'ext.popups' ] );
}
@ -153,8 +153,8 @@ class PopupsHooks {
* 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 @@ class PopupsHooks {
$module = PopupsContext::getInstance();
$user = $out->getUser();
$vars['wgPopupsIsEnabledByUser'] = $module->isEnabledByUser( $user );
$vars['wgPopupsShouldSendModuleToUser'] = $module->shouldSendModuleToUser( $user );
$vars['wgPopupsConflictsWithNavPopupGadget'] = $module->conflictsWithNavPopupsGadget(
$user );
}

View file

@ -147,7 +147,7 @@ class PopupsContext {
* @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 );

View file

@ -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' ) ) {

View file

@ -102,26 +102,26 @@ class PopupsContextTest extends MediaWikiTestCase {
}
/**
* @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 @@ class PopupsContextTest extends MediaWikiTestCase {
}
/**
* @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 @@ class PopupsContextTest extends MediaWikiTestCase {
$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 @@ class PopupsContextTest extends MediaWikiTestCase {
] );
$context = PopupsContext::getInstance();
$this->assertEquals( $expected, $context->isEnabledByUser( $user ) );
$this->assertEquals( $expected, $context->shouldSendModuleToUser( $user ) );
}
public static function providerAnonUserHasDisabledPagePreviews() {
@ -184,7 +184,7 @@ class PopupsContextTest extends MediaWikiTestCase {
/**
* @return array/
*/
public function provideTestDataForIsEnabledByUserWhenBetaEnabled() {
public function provideTestDataForShouldSendModuleToUserWhenBetaEnabled() {
return [
[
"optin" => PopupsContext::PREVIEWS_ENABLED,

View file

@ -274,7 +274,7 @@ class PopupsHooksTest extends MediaWikiTestCase {
* @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 @@ class PopupsHooksTest extends MediaWikiTestCase {
->with( [ 'ext.popups' ] );
$contextMock = $this->getMockBuilder( PopupsContextTestWrapper::class )
->setMethods( [ 'areDependenciesMet', 'isBetaFeatureEnabled', 'isEnabledByUser' ] )
->setMethods( [ 'areDependenciesMet', 'isBetaFeatureEnabled', 'shouldSendModuleToUser' ] )
->disableOriginalConstructor()
->getMock();
@ -303,8 +303,8 @@ class PopupsHooksTest extends MediaWikiTestCase {
->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 @@ class PopupsHooksTest extends MediaWikiTestCase {
->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 @@ class PopupsHooksTest extends MediaWikiTestCase {
PopupsHooks::onMakeGlobalVariablesScript( $vars, $outputPage );
$this->assertCount( 2, $vars );
$this->assertFalse( $vars[ 'wgPopupsIsEnabledByUser' ] );
$this->assertFalse( $vars[ 'wgPopupsShouldSendModuleToUser' ] );
$this->assertFalse( $vars[ 'wgPopupsConflictsWithNavPopupGadget' ] );
}
}

View file

@ -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 ),