Merge "Change the PopupsVisibility state to visible to match anon experience"

extension.json

@@ -43,6 +43,9 @@
 		],
 		"MakeGlobalVariablesScript": [
 			"Popups\\PopupsHooks::onMakeGlobalVariablesScript"
+		],
+		"LocalUserCreated": [
+			"Popups\\PopupsHooks::onLocalUserCreated"
 		]
 	},
 	"MessagesDirs": {
@@ -59,8 +62,10 @@
 		"PopupsVirtualPageViews": false,
 		"@PopupsHideOptInOnPreferencesPage": "@var bool: Whether the option to enable/disable Page Previews should be hidden on Preferences page. False by default",
 		"PopupsHideOptInOnPreferencesPage": false,
-		"@PopupsOptInDefaultState": "@var string:['1'|'0'] Default Page Previews visibility. Has to be a string as a compatibility with beta feature settings",
+		"@PopupsOptInDefaultState": "@var string:['1'|'0'] Default Page Previews visibility for old accounts. Has to be a string as a compatibility with beta feature settings. For more info see @T191888",
 		"PopupsOptInDefaultState": "0",
+		"@PopupsOptInStateForNewAccounts": "@var string:['1'|'0'] Default Page Previews visibility for newly created accounts (from Q2 2018). For more info see @T191888",
+		"PopupsOptInStateForNewAccounts": "0",
 		"@PopupsConflictingNavPopupsGadgetName": "@var string: Navigation popups gadget name",
 		"PopupsConflictingNavPopupsGadgetName": "Navigation_popups",
 		"@PopupsGateway": "Which gateway to use for fetching Popups data. Available options: [mwApiPlain|restbasePlain|restbaseHTML]. Full and always up to date list is available in src/gateway/index.js",

includes/PopupsContext.php

@@ -95,16 +95,6 @@ class PopupsContext {
 		return $this->gadgetsIntegration->conflictsWithNavPopupsGadget( $user );
 	}
 
-	/**
-	 * Get default Page previews state
-	 *
-	 * @see PopupsContext::PREVIEWS_ENABLED
-	 * @see PopupsContext::PREVIEWS_DISABLED
-	 * @return string
-	 */
-	public function getDefaultIsEnabledState() {
-		return $this->config->get( 'PopupsOptInDefaultState' );
-	}
 	/**
 	 * Are Page previews visible on User Preferences Page
 	 *

includes/PopupsHooks.php

@@ -139,9 +139,25 @@ class PopupsHooks {
 	 * @param array &$wgDefaultUserOptions Reference to default options array
 	 */
 	public static function onUserGetDefaultOptions( &$wgDefaultUserOptions ) {
-		$context = MediaWikiServices::getInstance()->getService( 'Popups.Context' );
+		$config = MediaWikiServices::getInstance()->getService( 'Popups.Config' );
+
 		$wgDefaultUserOptions[ PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME ] =
-			$context->getDefaultIsEnabledState();
+			$config->get( 'PopupsOptInDefaultState' );
 	}
 
+	/**
+	 * Change the default PagePreviews visibility state for newly created accounts
+	 *
+	 * @param User $user Newly created user object
+	 * @param bool $autocreated Is user autocreated
+	 */
+	public static function onLocalUserCreated( User $user, $autocreated ) {
+		if ( !$autocreated ) {
+			$config = MediaWikiServices::getInstance()->getService( 'Popups.Config' );
+
+			$user->setOption( PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME,
+				$config->get( 'PopupsOptInStateForNewAccounts' ) );
+			$user->saveSettings();
+		}
+	}
 }

tests/phpunit/PopupsContextTest.php

@@ -258,20 +258,6 @@ class PopupsContextTest extends MediaWikiTestCase {
 			'The title is blacklisted.' );
 	}
 
-	/**
-	 * @covers ::getDefaultIsEnabledState
-	 */
-	public function testGetDefaultIsEnabledState() {
-		$this->setMwGlobals( [
-			// Intentionally use a reserved value to verify it's returned.
-			'wgPopupsOptInDefaultState' => '2'
-		] );
-		$context = $this->getContext();
-		$this->assertEquals( '2',
-			$context->getDefaultIsEnabledState(),
-			'The value of PopupsOptInDefaultState is returned.' );
-	}
-
 	/**
 	 * @covers ::conflictsWithNavPopupsGadget
 	 */

tests/phpunit/PopupsHooksTest.php

@@ -186,31 +186,6 @@ class PopupsHooksTest extends MediaWikiTestCase {
 		}
 	}
 
-	/**
-	 * @covers ::onUserGetDefaultOptions
-	 */
-	public function testOnUserGetDefaultOptions() {
-		$userOptions = [
-			'test' => 'not_empty'
-		];
-		$contextMock = $this->getMockBuilder( PopupsContext::class )
-			->setMethods( [ 'getDefaultIsEnabledState' ] )
-			->disableOriginalConstructor()
-			->getMock();
-
-		$contextMock->expects( $this->once() )
-			->method( 'getDefaultIsEnabledState' )
-			->willReturn( true );
-
-		$this->setService( 'Popups.Context', $contextMock );
-
-		PopupsHooks::onUserGetDefaultOptions( $userOptions );
-		$this->assertCount( 2, $userOptions, 'A user option is retrieved.' );
-		$this->assertEquals( true,
-			$userOptions[ \Popups\PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME ],
-			'The opt-in preference is set.' );
-	}
-
 	/**
 	 * @covers ::onBeforePageDisplay
 	 */
@@ -335,4 +310,65 @@ class PopupsHooksTest extends MediaWikiTestCase {
 		$this->assertFalse( $vars[ 'wgPopupsConflictsWithNavPopupGadget' ],
 			'The PopupsConflictsWithNavPopupGadget global is present and false.' );
 	}
+
+	/**
+	 * @covers ::onUserGetDefaultOptions
+	 */
+	public function testOnUserGetDefaultOptions() {
+		$userOptions = [
+			'test' => 'not_empty'
+		];
+
+		$this->setMwGlobals( [
+			'wgPopupsOptInDefaultState' => "1"
+		] );
+
+		PopupsHooks::onUserGetDefaultOptions( $userOptions );
+		$this->assertCount( 2, $userOptions );
+		$this->assertEquals( "1",
+			$userOptions[ \Popups\PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME ] );
+	}
+
+	/**
+	 * @covers ::onUserGetDefaultOptions
+	 */
+	public function testOnLocalUserCreatedForAutoCreatedUser() {
+		$userMock =
+			$this->getMockBuilder( User::class )
+				->disableOriginalConstructor()
+				->setMethods( [ 'setOption', 'saveSettings' ] )
+				->getMock();
+
+		$userMock->expects( $this->never() )->method( 'setOption' );
+		$userMock->expects( $this->never() )->method( 'saveSettings' );
+
+		PopupsHooks::onLocalUserCreated( $userMock, true );
+	}
+
+	/**
+	 * @covers ::onUserGetDefaultOptions
+	 */
+	public function testOnLocalUserCreatedForNewlyCreatedUser() {
+		$expectedState = '1';
+
+		$userMock = $this->getMockBuilder( User::class )
+			->disableOriginalConstructor()
+			->setMethods( [ 'setOption', 'saveSettings' ] )
+			->getMock();
+		$userMock->expects( $this->once() )
+			->method( 'setOption' )
+			->with(
+				$this->equalTo( PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME ),
+				$this->equalTo( $expectedState )
+			);
+
+		$userMock->expects( $this->once() )
+			->method( 'saveSettings' );
+
+		$this->setMwGlobals( [
+			'wgPopupsOptInStateForNewAccounts' => $expectedState
+		] );
+		PopupsHooks::onLocalUserCreated( $userMock, false );
+	}
+
 }