Disable Page Previews preferences when NavPopups are enabled

Changes:
 - introduced PopupsGadgetsIntegration class handles all checks
 - show help message on Preferences page when NavPopups is enabled
 - unit test everything

Bug: T151058
Change-Id: Ia474b1b30378efe84dedf3ad47c1f833e88d69b5

Popups.hooks.php

@@ -50,9 +50,9 @@ class PopupsHooks {
 	 * @param array $prefs
 	 */
 	static function onGetPreferences( User $user, array &$prefs ) {
-		$module = PopupsContext::getInstance();
+		$context = PopupsContext::getInstance();
 
-		if ( !$module->showPreviewsOptInOnPreferencesPage() ) {
+		if ( !$context->showPreviewsOptInOnPreferencesPage() ) {
 			return;
 		}
 		$option = [
@@ -66,6 +66,11 @@ class PopupsHooks {
 			],
 			'section' => self::PREVIEWS_PREFERENCES_SECTION
 		];
+		if ( $context->conflictsWithNavPopupsGadget( $user ) ) {
+			$option[ 'disabled' ] = true;
+			$option[ 'help' ] = wfMessage( 'popups-prefs-disable-nav-gadgets-info',
+				'Special:Preferences#mw-prefsection-gadgets' );
+		}
 		$skinPosition = array_search( 'skin', array_keys( $prefs ) );
 
 		if ( $skinPosition !== false ) {

extension.json

@@ -10,7 +10,8 @@
 	"type": "betafeatures",
 	"AutoloadClasses": {
 		"PopupsHooks": "Popups.hooks.php",
-		"Popups\\PopupsContext": "includes/PopupsContext.php"
+		"Popups\\PopupsContext": "includes/PopupsContext.php",
+		"Popups\\PopupsGadgetsIntegration": "includes/PopupsGadgetsIntegration.php"
 	},
 	"ConfigRegistry": {
 		"popups": "GlobalVarConfig::newInstance"

i18n/en.json

@@ -24,5 +24,6 @@
 	"prefs-reading": "Reading preferences",
 	"popups-prefs-optin-title": "Page previews\n\n<em>Get quick previews of a topic while reading an article</em>",
 	"popups-prefs-optin-enabled-label": "Enable",
-	"popups-prefs-optin-disabled-label": "Disable"
+	"popups-prefs-optin-disabled-label": "Disable",
+	"popups-prefs-disable-nav-gadgets-info": "You have to [[$1 | disable Navigation Popups Gadget]] from Gadgets tab to enable Page Previews"
 }

i18n/qqq.json

@@ -30,5 +30,6 @@
 	"prefs-reading": "Title for 'Reading preferences' section on preferences page",
 	"popups-prefs-optin-title": "Title for Page Previews option\n\n<em>Description for Page previews option</em>",
 	"popups-prefs-optin-enabled-label": "Label for Page Previews opt in",
-	"popups-prefs-optin-disabled-label": "Label for Page previews opt out"
+	"popups-prefs-optin-disabled-label": "Label for Page previews opt out",
+	"popups-prefs-disable-nav-gadgets-info": "Help message telling to disable Navigation Popups gadget in order to enable Page Previews. Parameters: $1 - link to Preferences page"
 }

includes/PopupsContext.php

@@ -77,9 +77,12 @@ class PopupsContext {
 	 * Module constructor.
 	 * @param ExtensionRegistry $extensionRegistry
 	 */
-	protected function __construct( ExtensionRegistry $extensionRegistry ) {
+	protected function __construct( ExtensionRegistry $extensionRegistry,
+		PopupsGadgetsIntegration $gadgetsIntegration ) {
 		/** @todo Use MediaWikiServices Service Locator when it's ready */
 		$this->extensionRegistry = $extensionRegistry;
+		$this->gadgetsIntegration = $gadgetsIntegration;
+
 		$this->config = MediaWikiServices::getInstance()->getConfigFactory()
 			->makeConfig( PopupsContext::EXTENSION_NAME );
 	}
@@ -91,10 +94,20 @@ class PopupsContext {
 	 */
 	public static function getInstance() {
 		if ( !self::$instance ) {
-			self::$instance = new PopupsContext( ExtensionRegistry::getInstance() );
+			$registry = ExtensionRegistry::getInstance();
+			self::$instance = new PopupsContext( $registry,
+				new PopupsGadgetsIntegration( $registry ) );
 		}
 		return self::$instance;
 	}
+
+	/**
+	 * @param \User $user
+	 * @return bool
+	 */
+	public function conflictsWithNavPopupsGadget( \User $user ) {
+		return $this->gadgetsIntegration->conflictsWithNavPopupsGadget( $user );
+	}
 	/**
 	 * Is Beta Feature mode enabled
 	 *

includes/PopupsGadgetsIntegration.php

@@ -0,0 +1,67 @@
+<?php
+/*
+* This file is part of the MediaWiki extension Popups.
+*
+* Popups is free software: you can redistribute it and/or modify
+* it under the terms of the GNU General Public License as published by
+* the Free Software Foundation, either version 2 of the License, or
+* (at your option) any later version.
+*
+* Popups is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU General Public License for more details.
+*
+* You should have received a copy of the GNU General Public License
+* along with Popups.  If not, see <http://www.gnu.org/licenses/>.
+*
+* @file
+* @ingroup extensions
+*/
+namespace Popups;
+
+/**
+* Gadgets integration
+*
+* @package Popups
+*/
+class PopupsGadgetsIntegration {
+	const NAVIGATION_POPUPS_NAME = 'Navigation_popups';
+
+	/**
+	 * @var \ExtensionRegistry
+	 */
+	private $extensionRegistry;
+
+	/**
+	 * PopupsGadgetsIntegration constructor.
+	 * @param \ExtensionRegistry $extensionRegistry
+	 */
+	public function __construct( \ExtensionRegistry $extensionRegistry ) {
+		$this->extensionRegistry =  $extensionRegistry;
+	}
+
+	/**
+	 * @return bool
+	 */
+	private function isGadgetExtensionEnabled() {
+		return $this->extensionRegistry->isLoaded( 'Gadgets' );
+	}
+	/**
+	 * Check if Page Previews conflicts with Nav Popups Gadget
+	 * If user enabled Nav Popups PagePreviews are not available
+	 *
+	 * @param \User $user
+	 * @return bool
+	 */
+	public function conflictsWithNavPopupsGadget( \User $user ) {
+		if ( $this->isGadgetExtensionEnabled() ) {
+			$gadgetsRepo = \GadgetRepo::singleton();
+			$match = array_search( self::NAVIGATION_POPUPS_NAME, $gadgetsRepo->getGadgetIds() );
+			if ( $match !== false ) {
+				return $gadgetsRepo->getGadget( self::NAVIGATION_POPUPS_NAME )->isEnabled( $user );
+			}
+		}
+		return false;
+	}
+}

tests/phpunit/PopupsContextTest.php

@@ -280,4 +280,24 @@ class PopupsContextTest extends MediaWikiTestCase {
 		] );
 		$this->assertEquals( "2", PopupsContext::getInstance()->getDefaultIsEnabledState() );
 	}
+
+	/**
+	 * @covers ::conflictsWithNavPopupsGadget
+	 */
+	public function testConflictsWithNavPopupsGadget() {
+		$integrationMock = $this->getMockBuilder( \Popups\PopupsGadgetsIntegration::class )
+			->disableOriginalConstructor()
+			->setMethods( [ 'conflictsWithNavPopupsGadget' ] )
+			->getMock();
+
+		$user = $this->getTestUser()->getUser();
+
+		$integrationMock->expects( $this->once() )
+			->method( 'conflictsWithNavPopupsGadget' )
+			->with( $user )
+			->willReturn( true );
+
+		$context = new PopupsContextTestWrapper( ExtensionRegistry::getInstance(), $integrationMock );
+		$this->assertEquals( true, $context->conflictsWithNavPopupsGadget( $user ) );
+	}
 }

tests/phpunit/PopupsContextTestWrapper.php

@@ -31,15 +31,23 @@ use \Popups\PopupsContext;
  * Used for testing only
  * @codeCoverageIgnore
  */
+use Popups\PopupsGadgetsIntegration;
+
 class PopupsContextTestWrapper extends PopupsContext {
 
 	/**
 	 * Override constructor so we can create new instances for testing
-	 * .
+	 *
 	 * @param ExtensionRegistry $extensionRegistry
+	 * @param PopupsGadgetsIntegration $gadgetsIntegration
 	 */
-	public function __construct( ExtensionRegistry $extensionRegistry ) {
-		parent::__construct( $extensionRegistry );
+	public function __construct( ExtensionRegistry $extensionRegistry,
+		PopupsGadgetsIntegration $gadgetsIntegration = null ) {
+
+		$gadgetsIntegration = $gadgetsIntegration ? $gadgetsIntegration :
+			new PopupsGadgetsIntegration( $extensionRegistry );
+
+		parent::__construct( $extensionRegistry, $gadgetsIntegration );
 	}
 
 	/**

tests/phpunit/PopupsGadgetsIntegrationTest.php

@@ -0,0 +1,141 @@
+<?php
+/*
+* This file is part of the MediaWiki extension Popups.
+*
+* Popups is free software: you can redistribute it and/or modify
+* it under the terms of the GNU General Public License as published by
+* the Free Software Foundation, either version 2 of the License, or
+* (at your option) any later version.
+*
+* Popups is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU General Public License for more details.
+*
+* You should have received a copy of the GNU General Public License
+* along with Popups.  If not, see <http://www.gnu.org/licenses/>.
+*
+* @file
+* @ingroup extensions
+*/
+
+use Popups\PopupsGadgetsIntegration;
+
+/**
+* Popups module tests
+*
+* @group Popups
+* @coversDefaultClass Popups\PopupsGadgetsIntegration
+*/
+class PopupsGadgetsIntegrationTest extends MediaWikiTestCase
+{
+	/**
+	 * Helper constants for easier reading
+	 */
+	const GADGET_ENABLED = true;
+	/**
+	 * Helper constants for easier reading
+	 */
+	const GADGET_DISABLED = false;
+
+	/**
+	 * Checks if Gadgets extension is available
+	 */
+	private function checkRequiredDependencies() {
+		if ( !ExtensionRegistry::getInstance()->isLoaded( 'Gadgets' ) ) {
+			$this->markTestSkipped( 'Skipped as Gadgets extension is not available' );
+		}
+	}
+	/**
+	 * @param bool $gadgetsEnabled
+	 * @return ExtensionRegistry
+	 */
+	private function getExtensionRegistryMock( $gadgetsEnabled ) {
+		$mock = $this->getMock( ExtensionRegistry::class, [ 'isLoaded' ] );
+		$mock->expects( $this->any() )
+			->method( 'isLoaded' )
+			->with( 'Gadgets' )
+			->willReturn( $gadgetsEnabled );
+		return $mock;
+	}
+	/**
+	 * @covers ::conflictsWithNavPopupsGadget
+	 * @covers ::isGadgetExtensionEnabled
+	 * @covers ::__construct
+	 */
+	public function testConflictsWithNavPopupsGadgetIfGadgetsExtensionIsNotLoaded() {
+		$user = $this->getTestUser()->getUser();
+		$integration = new PopupsGadgetsIntegration( $this->getExtensionRegistryMock( false ) );
+		$this->assertEquals( false, $integration->conflictsWithNavPopupsGadget( $user ) );
+	}
+
+	/**
+	 * @covers ::conflictsWithNavPopupsGadget
+	 */
+	public function testConflictsWithNavPopupsGadgetIfGadgetNotExists() {
+		$this->checkRequiredDependencies();
+
+		$user = $this->getTestUser()->getUser();
+
+		$gadgetRepoMock = $this->getMock( GadgetRepo::class, [ 'getGadgetIds', 'getGadget' ] );
+
+		$gadgetRepoMock->expects( $this->once() )
+			->method( 'getGadgetIds' )
+			->willReturn( [] );
+
+		$this->executeConflictsWithNavPopupsGadgetSafeCheck( $user, $gadgetRepoMock,
+			self::GADGET_DISABLED );
+	}
+
+	/**
+	 * @covers ::conflictsWithNavPopupsGadget
+	 */
+	public function testConflictsWithNavPopupsGadgetIfGadgetExists() {
+		$this->checkRequiredDependencies();
+
+		$user = $this->getTestUser()->getUser();
+
+		$gadgetMock = $this->getMockBuilder( Gadget::class )
+			->setMethods( [ 'isEnabled', 'getGadget' ] )
+			->disableOriginalConstructor()
+			->getMock();
+
+		$gadgetMock->expects( $this->once() )
+			->method( 'isEnabled' )
+			->with( $user )
+			->willReturn( self::GADGET_ENABLED );
+
+		$gadgetRepoMock = $this->getMock( GadgetRepo::class,
+			[ 'getGadgetIds', 'getGadget' ] );
+
+		$gadgetRepoMock->expects( $this->once() )
+			->method( 'getGadgetids' )
+			->willReturn( [ 'Navigation_popups' ] );
+
+		$gadgetRepoMock->expects( $this->once() )
+			->method( 'getGadget' )
+			->with( 'Navigation_popups' )
+			->willReturn( $gadgetMock );
+
+		$this->executeConflictsWithNavPopupsGadgetSafeCheck( $user, $gadgetRepoMock,
+			self::GADGET_ENABLED );
+	}
+
+	/**
+	 * Execute test and restore orGadgetRepo
+	 *
+	 * @param $user
+	 * @param $repoMock
+	 * @param $expected
+	 */
+	private function executeConflictsWithNavPopupsGadgetSafeCheck( $user, $repoMock, $expected ) {
+		$origGadgetsRepo = GadgetRepo::singleton();
+		GadgetRepo::setSingleton( $repoMock );
+
+		$integration = new PopupsGadgetsIntegration( $this->getExtensionRegistryMock( true ) );
+		$this->assertEquals( $expected, $integration->conflictsWithNavPopupsGadget( $user ) );
+
+		GadgetRepo::setSingleton( $origGadgetsRepo );
+	}
+
+}

tests/phpunit/PopupsHooksTest.php

@@ -19,6 +19,7 @@
  * @ingroup extensions
  */
 require_once ( 'PopupsContextTestWrapper.php' );
+use Popups\PopupsContext;
 
 /**
  * Integration tests for Page Preview hooks
@@ -54,7 +55,7 @@ class PopupsHooksTest extends MediaWikiTestCase {
 
 		PopupsHooks::onGetBetaPreferences( $this->getTestUser()->getUser(), $prefs );
 		$this->assertCount( 2, $prefs );
-		$this->assertArrayHasKey( \Popups\PopupsContext::PREVIEWS_BETA_PREFERENCE_NAME, $prefs );
+		$this->assertArrayHasKey( PopupsContext::PREVIEWS_BETA_PREFERENCE_NAME, $prefs );
 	}
 
 	/**
@@ -75,6 +76,36 @@ class PopupsHooksTest extends MediaWikiTestCase {
 		$this->assertEquals( 'notEmpty', $prefs[ 'someNotEmptyValue'] );
 	}
 
+	/**
+	 * @covers ::onGetPreferences
+	 */
+	public function testOnGetPreferencesNavPopupGadgetIsOn() {
+		$userMock = $this->getTestUser()->getUser();
+		$contextMock = $this->getMock( PopupsContextTestWrapper::class,
+			[ 'showPreviewsOptInOnPreferencesPage', 'conflictsWithNavPopupsGadget' ],
+			[ ExtensionRegistry::getInstance() ] );
+
+		$contextMock->expects( $this->once() )
+			->method( 'showPreviewsOptInOnPreferencesPage' )
+			->will( $this->returnValue( true ) );
+
+		$contextMock->expects( $this->once() )
+			->method( 'conflictsWithNavPopupsGadget' )
+			->with( $userMock )
+			->will( $this->returnValue( true ) );
+
+		PopupsContextTestWrapper::injectTestInstance( $contextMock );
+		$prefs = [];
+
+		PopupsHooks::onGetPreferences( $this->getTestUser()->getUser(), $prefs );
+		$this->assertArrayHasKey( PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME, $prefs );
+		$this->assertArrayHasKey( 'disabled',
+			$prefs[ PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME ] );
+		$this->assertEquals( true,
+			$prefs[ PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME]['disabled'] );
+		$this->assertNotEmpty( $prefs[ PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME]['help'] );
+	}
+
 	/**
 	 * @covers ::onGetPreferences
 	 */
@@ -95,7 +126,7 @@ class PopupsHooksTest extends MediaWikiTestCase {
 		PopupsHooks::onGetPreferences( $this->getTestUser()->getUser(), $prefs );
 		$this->assertCount( 4, $prefs );
 		$this->assertEquals( 'notEmpty', $prefs[ 'someNotEmptyValue'] );
-		$this->assertArrayHasKey( \Popups\PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME, $prefs );
+		$this->assertArrayHasKey( PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME, $prefs );
 		$this->assertEquals( 1, array_search( \Popups\PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME,
 			array_keys( $prefs ) ), ' PagePreviews preferences should be injected after Skin select' );
 	}
@@ -118,8 +149,8 @@ class PopupsHooksTest extends MediaWikiTestCase {
 
 		PopupsHooks::onGetPreferences( $this->getTestUser()->getUser(), $prefs );
 		$this->assertCount( 3, $prefs );
-		$this->assertArrayHasKey( \Popups\PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME, $prefs );
-		$this->assertEquals( 2, array_search( \Popups\PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME,
+		$this->assertArrayHasKey( PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME, $prefs );
+		$this->assertEquals( 2, array_search( PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME,
 			array_keys( $prefs ) ), ' PagePreviews should be injected at end of array' );
 	}