From f0d7406811ee6b880bfb3683317ad1129595703d Mon Sep 17 00:00:00 2001
From: WMDE-Fisch <christoph.jauera@wikimedia.de>
Date: Fri, 26 Apr 2024 20:05:40 +0200
Subject: [PATCH] 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                       | 47 +++++++++--------
 .../ReferencePreviewsContext.php              |  2 +
 tests/phpunit/CiteHooksTest.php               | 50 ++++++++++++++++---
 3 files changed, 70 insertions(+), 29 deletions(-)

diff --git a/src/Hooks/CiteHooks.php b/src/Hooks/CiteHooks.php
index ac2c9fe99..88547fb19 100644
--- a/src/Hooks/CiteHooks.php
+++ b/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' ) ) {
-			$dir = dirname( __DIR__, 2 ) . '/modules/';
-			$resourceLoader->register( [
-				'ext.cite.referencePreviews' => [
-					'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'
-					]
-				]
-			] );
+		if ( !$resourceLoader->getConfig()->get( 'CiteReferencePreviews' ) ||
+			!ExtensionRegistry::getInstance()->isLoaded( 'Popups' )
+		) {
+			return;
 		}
+
+		$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'
+				]
+			]
+		] );
 	}
 
 	/**
diff --git a/src/ReferencePreviews/ReferencePreviewsContext.php b/src/ReferencePreviews/ReferencePreviewsContext.php
index 8fe26a1bf..3e7f9bee3 100644
--- a/src/ReferencePreviews/ReferencePreviewsContext.php
+++ b/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 )
diff --git a/tests/phpunit/CiteHooksTest.php b/tests/phpunit/CiteHooksTest.php
index 3e1e51e44..4918238fb 100644
--- a/tests/phpunit/CiteHooksTest.php
+++ b/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->assertArrayHasKey( 'wgCiteResponsiveReferences', $vars );
+		$this->assertSame( [
+			'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 ];
 	}
 
 }