Merge "Respect useskin when operating in MigrationMode"

includes/Constants.php

@@ -162,6 +162,12 @@ final class Constants {
 	 */
 	public const QUERY_PARAM_SKIN_VERSION = 'useskinversion';
 
+	/**
+	 * Override the skin user preference and site Config. See readme.
+	 * @var string
+	 */
+	public const QUERY_PARAM_SKIN = 'useskin';
+
 	/**
 	 * @var string
 	 */

includes/SkinVersionLookup.php

@@ -107,6 +107,19 @@ final class SkinVersionLookup {
 	 * @throws \ConfigException
 	 */
 	public function getVersion(): string {
+		$migrationMode = $this->config->get( 'VectorSkinMigrationMode' );
+		// In migration mode, the useskin parameter is the source of truth.
+		if ( $migrationMode ) {
+			$useSkin = $this->request->getVal(
+				Constants::QUERY_PARAM_SKIN
+			);
+			if ( $useSkin ) {
+				return $useSkin === Constants::SKIN_NAME_LEGACY ?
+					Constants::SKIN_VERSION_LEGACY :
+					Constants::SKIN_VERSION_LATEST;
+			}
+		}
+
 		// If skin key is not vector, then version should be considered legacy.
 
 		// If skin is "Vector" invoke additional skin versioning detection.
@@ -138,9 +151,12 @@ final class SkinVersionLookup {
 			)
 		);
 
-		// If we are in migration mode, we must check the skin version preference.
-		if ( $this->config->get( 'VectorSkinMigrationMode' ) ) {
+		// If we are in migration mode...
+		if ( $migrationMode ) {
+			// ... we must check the skin version preference for logged in users.
+			// No need to check for anons as wgDefaultSkin has already been consulted at this point.
 			if (
+				$this->user->isRegistered() &&
 				$skin === Constants::SKIN_NAME_LEGACY &&
 				$skinVersionPref === Constants::SKIN_VERSION_LATEST
 			) {

tests/phpunit/integration/SkinVersionLookupTest.php

@@ -148,6 +148,114 @@ class SkinVersionLookupTest extends \MediaWikiIntegrationTestCase {
 		);
 	}
 
+	public function providerAnonUserMigrationMode() {
+		return [
+			// When no query string just return DefaultSkin version.
+			[
+				Constants::SKIN_NAME_LEGACY,
+				null,
+				Constants::SKIN_VERSION_LEGACY,
+			],
+			[
+				Constants::SKIN_NAME_MODERN,
+				null,
+				Constants::SKIN_VERSION_LATEST,
+			],
+			// When useskin=vector return legacy Vector version.
+			[
+				Constants::SKIN_NAME_LEGACY,
+				Constants::SKIN_NAME_LEGACY,
+				Constants::SKIN_VERSION_LEGACY,
+			],
+			[
+				Constants::SKIN_NAME_MODERN,
+				Constants::SKIN_NAME_LEGACY,
+				Constants::SKIN_VERSION_LEGACY,
+			],
+			// When useskin=vector-2022 return modern Vector.
+			[
+				Constants::SKIN_NAME_MODERN,
+				Constants::SKIN_NAME_MODERN,
+				Constants::SKIN_VERSION_LATEST,
+			],
+			[
+				Constants::SKIN_NAME_LEGACY,
+				Constants::SKIN_NAME_MODERN,
+				Constants::SKIN_VERSION_LATEST,
+			],
+		];
+	}
+
+	/**
+	 * @covers ::getVersion
+	 * @dataProvider providerAnonUserMigrationMode
+	 */
+	public function testVectorAnonUserMigrationModeWithUseSkinVector(
+		string $defaultSkin,
+		$useSkin,
+		$expectedVersion
+	) {
+		$request = $this->getMockBuilder( \WebRequest::class )->getMock();
+		$request
+			->method( 'getVal' )
+			->with( 'useskin' )
+			->willReturn( $useSkin );
+		$user = $this->createMock( \User::class );
+		$user
+			->method( 'isRegistered' )
+			->willReturn( false );
+
+		$config = new HashConfig( [
+			'DefaultSkin' => $defaultSkin,
+			'VectorSkinMigrationMode' => true,
+			'VectorDefaultSkinVersion' => '2',
+			'VectorDefaultSkinVersionForExistingAccounts' => '2'
+		] );
+		$userOptionsLookup = $this->getUserOptionsLookupMock( $user, '2', [
+			'skin' => $defaultSkin,
+		] );
+
+		$skinVersionLookup = new SkinVersionLookup( $request, $user, $config, $userOptionsLookup );
+
+		$this->assertSame(
+			$expectedVersion,
+			$skinVersionLookup->getVersion(),
+			'useskin=vector query string yields legacy skin in migration mode'
+		);
+	}
+
+	/**
+	 * @covers ::getVersion
+	 */
+	public function testVectorRegisteredUserMigrationMode() {
+		$request = $this->getMockBuilder( \WebRequest::class )->getMock();
+		$request
+			->method( 'getVal' )
+			->willReturn( null );
+		$user = $this->createMock( \User::class );
+		$user
+			->method( 'isRegistered' )
+			->willReturn( true );
+
+		$config = new HashConfig( [
+			'DefaultSkin' => 'vector',
+			'VectorSkinMigrationMode' => true,
+			'VectorDefaultSkinVersion' => '1',
+			'VectorDefaultSkinVersionForExistingAccounts' => '1'
+		] );
+		$userOptionsLookup = $this->getUserOptionsLookupMock( $user, '2', [
+			'skin' => Constants::SKIN_NAME_LEGACY
+		] );
+
+		$skinVersionLookup = new SkinVersionLookup( $request, $user, $config, $userOptionsLookup );
+
+		$this->assertSame(
+			'2',
+			$skinVersionLookup->getVersion(),
+			'If legacy skin is set with skin version modern, then the user gets modern skin still'
+		);
+	}
+
 	/**
 	 * @covers ::getVersion
 	 */