Don't load ReferencePreviews when not enabled in the config

Includes a test and some improvements to the CiteHooks tests.

Change-Id: I0e9108d0d429ed9b7467f993441eefc2557c8e6f

src/Hooks/CiteHooks.php

@@ -92,29 +92,32 @@ class CiteHooks implements
 	 * @see https://www.mediawiki.org/wiki/Manual:Hooks/ResourceLoaderRegisterModules
 	 */
 	public function onResourceLoaderRegisterModules( ResourceLoader $resourceLoader ): void {
-		if ( ExtensionRegistry::getInstance()->isLoaded( 'Popups' ) ) {
+		if ( !$resourceLoader->getConfig()->get( 'CiteReferencePreviews' ) ||
-			$dir = dirname( __DIR__, 2 ) . '/modules/';
+			!ExtensionRegistry::getInstance()->isLoaded( 'Popups' )
-			$resourceLoader->register( [
+		) {
-				'ext.cite.referencePreviews' => [
+			return;
-					'localBasePath' => $dir . '/ext.cite.referencePreviews',
-					'remoteExtPath' => 'Cite/modules/ext.cite.referencePreviews',
-					'dependencies' => [
-						'ext.popups.main',
-					],
-					'styles' => [
-						'referencePreview.less',
-					],
-					'packageFiles' => [
-						'index.js',
-						'constants.js',
-						'createReferenceGateway.js',
-						'createReferencePreview.js',
-						'isReferencePreviewsEnabled.js',
-						'referencePreviewsInstrumentation.js'
-					]
-				]
-			] );
 		}
+
+		$resourceLoader->register( [
+			'ext.cite.referencePreviews' => [
+				'localBasePath' => dirname( __DIR__, 2 ) . '/modules/ext.cite.referencePreviews',
+				'remoteExtPath' => 'Cite/modules/ext.cite.referencePreviews',
+				'dependencies' => [
+					'ext.popups.main',
+				],
+				'styles' => [
+					'referencePreview.less',
+				],
+				'packageFiles' => [
+					'index.js',
+					'constants.js',
+					'createReferenceGateway.js',
+					'createReferencePreview.js',
+					'isReferencePreviewsEnabled.js',
+					'referencePreviewsInstrumentation.js'
+				]
+			]
+		] );
 	}
 
 	/**

src/ReferencePreviews/ReferencePreviewsContext.php

@@ -38,6 +38,8 @@ class ReferencePreviewsContext {
 		if (
 			// T243822: Temporarily disabled in the mobile skin
 			$skin->getSkinName() === 'minerva' ||
+			// The feature flag is also checked in the ResourceLoaderRegisterModules hook handler
+			// and technically redundant here, but it's cheap; better safe than sorry
 			!$this->config->get( 'CiteReferencePreviews' ) ||
 			$this->gadgetsIntegration->isRefToolTipsGadgetEnabled( $user ) ||
 			$this->gadgetsIntegration->isNavPopupsGadgetEnabled( $user )

tests/phpunit/CiteHooksTest.php

@@ -4,6 +4,8 @@ namespace Cite\Tests;
 
 use ApiQuerySiteinfo;
 use Cite\Hooks\CiteHooks;
+use MediaWiki\Config\HashConfig;
+use MediaWiki\ResourceLoader\ResourceLoader;
 use MediaWiki\User\Options\StaticUserOptionsLookup;
 
 /**
@@ -12,30 +14,64 @@ use MediaWiki\User\Options\StaticUserOptionsLookup;
  */
 class CiteHooksTest extends \MediaWikiIntegrationTestCase {
 
-	public function testOnResourceLoaderGetConfigVars() {
+	/**
+	 * @dataProvider provideBooleans
+	 */
+	public function testOnResourceLoaderGetConfigVars( bool $enabled ) {
 		$vars = [];
 
-		$config = $this->getServiceContainer()->getMainConfig();
+		$config = new HashConfig( [
+			'CiteVisualEditorOtherGroup' => $enabled,
+			'CiteResponsiveReferences' => $enabled,
+			'CiteBookReferencing' => $enabled,
+		] );
 
 		$citeHooks = new CiteHooks( new StaticUserOptionsLookup( [] ) );
 		$citeHooks->onResourceLoaderGetConfigVars( $vars, 'vector', $config );
 
-		$this->assertArrayHasKey( 'wgCiteVisualEditorOtherGroup', $vars );
+		$this->assertSame( [
-		$this->assertArrayHasKey( 'wgCiteResponsiveReferences', $vars );
+			'wgCiteVisualEditorOtherGroup' => $enabled,
+			'wgCiteResponsiveReferences' => $enabled,
+			'wgCiteBookReferencing' => $enabled,
+		], $vars );
 	}
 
-	public function testOnAPIQuerySiteInfoGeneralInfo() {
+	/**
+	 * @dataProvider provideBooleans
+	 */
+	public function testOnResourceLoaderRegisterModules( bool $enabled ) {
+		$this->markTestSkippedIfExtensionNotLoaded( 'Popups' );
+
+		$resourceLoader = $this->createMock( ResourceLoader::class );
+		$resourceLoader->method( 'getConfig' )
+			->willReturn( new HashConfig( [ 'CiteReferencePreviews' => $enabled ] ) );
+		$resourceLoader->expects( $this->exactly( (int)$enabled ) )
+			->method( 'register' );
+
+		$citeHooks = new CiteHooks( new StaticUserOptionsLookup( [] ) );
+		$citeHooks->onResourceLoaderRegisterModules( $resourceLoader );
+	}
+
+	/**
+	 * @dataProvider provideBooleans
+	 */
+	public function testOnAPIQuerySiteInfoGeneralInfo( bool $enabled ) {
 		$api = $this->createMock( ApiQuerySiteinfo::class );
 		$api->expects( $this->once() )
 			->method( 'getConfig' )
-			->willReturn( $this->getServiceContainer()->getMainConfig() );
+			->willReturn( new HashConfig( [ 'CiteResponsiveReferences' => $enabled ] ) );
 
 		$data = [];
 
 		$citeHooks = new CiteHooks( new StaticUserOptionsLookup( [] ) );
 		$citeHooks->onAPIQuerySiteInfoGeneralInfo( $api, $data );
 
-		$this->assertArrayHasKey( 'citeresponsivereferences', $data );
+		$this->assertSame( [ 'citeresponsivereferences' => $enabled ], $data );
+	}
+
+	public static function provideBooleans() {
+		yield [ true ];
+		yield [ false ];
 	}
 
 }