From 7a796022087103f1fb61e7b0e40f6e430aefcbc7 Mon Sep 17 00:00:00 2001 From: Adam Wight Date: Wed, 4 Sep 2024 12:33:07 +0200 Subject: [PATCH] [cleanup] Remove unused configuration $wgPopupsReferencePreviews This configuration is always enabled, so remaining conditions are dead code. Removing the flag in this code base simplifies moving the remaining Reference Previews settings in a later patch. Bug: T363162 Change-Id: I2b952f4203b6ffa040daad2aa288eb53d2ffd3b2 --- extension.json | 4 --- includes/PopupsContext.php | 4 --- includes/PopupsHooks.php | 17 ++++------- tests/phpunit/PopupsContextTest.php | 40 -------------------------- tests/phpunit/PopupsHooksTest.php | 44 ++++++++--------------------- 5 files changed, 17 insertions(+), 92 deletions(-) diff --git a/extension.json b/extension.json index 3b3181e2d..bfa4fa82f 100644 --- a/extension.json +++ b/extension.json @@ -78,10 +78,6 @@ "description": "Specify a REST endpoint where summaries should be sourced from. Endpoint must meet the spec at https://www.mediawiki.org/wiki/Specs/Summary/1.2.0", "value": "/api/rest_v1/page/summary/" }, - "PopupsReferencePreviews": { - "description": "Feature flag to enable or disable the separate popup type for references created via the Cite extension's tag.", - "value": true - }, "PopupsStatsvSamplingRate": { "description": "Sampling rate for logging performance data to statsv.", "value": 0 diff --git a/includes/PopupsContext.php b/includes/PopupsContext.php index a3d446aaf..ad40e4be8 100644 --- a/includes/PopupsContext.php +++ b/includes/PopupsContext.php @@ -150,10 +150,6 @@ class PopupsContext { * @return bool whether or not to show reference previews */ public function isReferencePreviewsEnabled( User $user ) { - if ( !$this->config->get( 'PopupsReferencePreviews' ) ) { - return false; - } - return !$user->isNamed() || $this->userOptionsLookup->getBoolOption( $user, self::REFERENCE_PREVIEWS_PREFERENCE_NAME ); diff --git a/includes/PopupsHooks.php b/includes/PopupsHooks.php index 53dd548f1..0308d56c5 100644 --- a/includes/PopupsHooks.php +++ b/includes/PopupsHooks.php @@ -101,14 +101,10 @@ class PopupsHooks implements } $skinPosition = array_search( 'skin', array_keys( $prefs ) ); - $readingOptions = $this->getPagePreviewPrefToggle( $user ); - - if ( $this->config->get( 'PopupsReferencePreviews' ) ) { - $readingOptions = array_merge( - $readingOptions, - $this->getReferencePreviewPrefToggle( $user ) - ); - } + $readingOptions = array_merge( + $this->getPagePreviewPrefToggle( $user ), + $this->getReferencePreviewPrefToggle( $user ) + ); if ( $skinPosition !== false ) { $injectIntoIndex = $skinPosition + 1; @@ -251,9 +247,6 @@ class PopupsHooks implements public function onUserGetDefaultOptions( &$defaultOptions ) { $default = $this->config->get( 'PopupsOptInDefaultState' ); $defaultOptions[PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME] = $default; - - if ( $this->config->get( 'PopupsReferencePreviews' ) ) { - $defaultOptions[PopupsContext::REFERENCE_PREVIEWS_PREFERENCE_NAME] = '1'; - } + $defaultOptions[PopupsContext::REFERENCE_PREVIEWS_PREFERENCE_NAME] = '1'; } } diff --git a/tests/phpunit/PopupsContextTest.php b/tests/phpunit/PopupsContextTest.php index d8e08f81d..2aa292ca1 100644 --- a/tests/phpunit/PopupsContextTest.php +++ b/tests/phpunit/PopupsContextTest.php @@ -22,7 +22,6 @@ use MediaWiki\Config\GlobalVarConfig; use MediaWiki\MainConfigNames; use MediaWiki\Title\Title; -use MediaWiki\User\Options\UserOptionsLookup; use MediaWiki\User\User; use PHPUnit\Framework\MockObject\Stub\ConsecutiveCalls; use Popups\PopupsContext; @@ -109,45 +108,6 @@ class PopupsContextTest extends MediaWikiIntegrationTestCase { ); } - /** - * Tests #shouldSendModuleToUser when the user is logged in and the reference previews feature - * is disabled. - * - * @covers ::shouldSendModuleToUser - * @dataProvider provideTestDataForShouldSendModuleToUser - * @param bool $optIn - * @param bool $expected - */ - public function testShouldSendModuleToUser( $optIn, $expected ) { - $this->overrideConfigValue( 'PopupsReferencePreviews', false ); - - $user = $this->createMock( User::class ); - $user->method( 'isNamed' )->willReturn( true ); - $userOptLookup = $this->createMock( UserOptionsLookup::class ); - $userOptLookup->method( 'getBoolOption' ) - ->with( $user, PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME ) - ->willReturn( $optIn ); - $this->setService( 'UserOptionsLookup', $userOptLookup ); - - $context = $this->getContext(); - $this->assertSame( $expected, - $context->shouldSendModuleToUser( $user ), - ( $expected ? 'A' : 'No' ) . ' module is sent to the user.' ); - } - - public static function provideTestDataForShouldSendModuleToUser() { - return [ - [ - 'optin' => PopupsContext::PREVIEWS_ENABLED, - 'expected' => true - ], - [ - 'optin' => PopupsContext::PREVIEWS_DISABLED, - 'expected' => false - ] - ]; - } - /** * @covers ::areDependenciesMet * @covers ::__construct diff --git a/tests/phpunit/PopupsHooksTest.php b/tests/phpunit/PopupsHooksTest.php index 584efec79..776773c39 100644 --- a/tests/phpunit/PopupsHooksTest.php +++ b/tests/phpunit/PopupsHooksTest.php @@ -61,16 +61,15 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase { /** * @covers ::onGetPreferences - * @dataProvider provideReferencePreviewsFlag */ - public function testOnGetPreferencesNavPopupGadgetIsOn( bool $enabled ) { + public function testOnGetPreferencesNavPopupGadgetIsOn() { $userMock = $this->createMock( User::class ); $contextMock = $this->createMock( PopupsContext::class ); $contextMock->expects( $this->once() ) ->method( 'showPreviewsOptInOnPreferencesPage' ) ->willReturn( true ); - $contextMock->expects( $this->exactly( $enabled ? 2 : 1 ) ) + $contextMock->expects( $this->exactly( 2 ) ) ->method( 'conflictsWithNavPopupsGadget' ) ->with( $userMock ) ->willReturn( true ); @@ -78,9 +77,7 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase { $prefs = []; ( new PopupsHooks( - new HashConfig( [ - 'PopupsReferencePreviews' => $enabled, - ] ), + new HashConfig(), $contextMock, $this->getServiceContainer()->getService( 'Popups.Logger' ), $this->getServiceContainer()->getUserOptionsManager() @@ -101,14 +98,13 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase { /** * @covers ::onGetPreferences - * @dataProvider provideReferencePreviewsFlag */ - public function testOnGetPreferencesPreviewsEnabled( bool $enabled ) { + public function testOnGetPreferencesPreviewsEnabled() { $contextMock = $this->createMock( PopupsContext::class ); $contextMock->expects( $this->once() ) ->method( 'showPreviewsOptInOnPreferencesPage' ) ->willReturn( true ); - $contextMock->expects( $this->exactly( $enabled ? 2 : 1 ) ) + $contextMock->expects( $this->exactly( 2 ) ) ->method( 'conflictsWithNavPopupsGadget' ) ->willReturn( false ); @@ -119,9 +115,7 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase { ]; ( new PopupsHooks( - new HashConfig( [ - 'PopupsReferencePreviews' => $enabled, - ] ), + new HashConfig(), $contextMock, $this->getServiceContainer()->getService( 'Popups.Logger' ), $this->getServiceContainer()->getUserOptionsManager() @@ -142,14 +136,13 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase { /** * @covers ::onGetPreferences - * @dataProvider provideReferencePreviewsFlag */ - public function testOnGetPreferencesPreviewsEnabledWhenSkinIsNotAvailable( bool $enabled ) { + public function testOnGetPreferencesPreviewsEnabledWhenSkinIsNotAvailable() { $contextMock = $this->createMock( PopupsContext::class ); $contextMock->expects( $this->once() ) ->method( 'showPreviewsOptInOnPreferencesPage' ) ->willReturn( true ); - $contextMock->expects( $this->exactly( $enabled ? 2 : 1 ) ) + $contextMock->expects( $this->exactly( 2 ) ) ->method( 'conflictsWithNavPopupsGadget' ) ->willReturn( false ); @@ -159,9 +152,7 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase { ]; ( new PopupsHooks( - new HashConfig( [ - 'PopupsReferencePreviews' => $enabled, - ] ), + new HashConfig(), $contextMock, $this->getServiceContainer()->getService( 'Popups.Logger' ), $this->getServiceContainer()->getUserOptionsManager() @@ -317,9 +308,8 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase { /** * @covers ::onUserGetDefaultOptions - * @dataProvider provideReferencePreviewsFlag */ - public function testOnUserGetDefaultOptions( bool $enabled ) { + public function testOnUserGetDefaultOptions() { $userOptions = [ 'test' => 'not_empty' ]; @@ -327,25 +317,15 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase { ( new PopupsHooks( new HashConfig( [ 'PopupsOptInDefaultState' => '1', - 'PopupsReferencePreviews' => $enabled, ] ), $this->getServiceContainer()->getService( 'Popups.Context' ), $this->getServiceContainer()->getService( 'Popups.Logger' ), $this->getServiceContainer()->getUserOptionsManager() ) ) ->onUserGetDefaultOptions( $userOptions ); - $this->assertCount( $enabled ? 3 : 2, $userOptions ); + $this->assertCount( 3, $userOptions ); $this->assertSame( '1', $userOptions[ PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME ] ); - if ( $enabled ) { - $this->assertSame( '1', $userOptions[ PopupsContext::REFERENCE_PREVIEWS_PREFERENCE_NAME ] ); - } - } - - public static function provideReferencePreviewsFlag() { - return [ - [ false ], - [ true ], - ]; + $this->assertSame( '1', $userOptions[ PopupsContext::REFERENCE_PREVIEWS_PREFERENCE_NAME ] ); } }