diff --git a/includes/Constants.php b/includes/Constants.php index ecb94885..008f3d78 100644 --- a/includes/Constants.php +++ b/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 */ diff --git a/includes/SkinVersionLookup.php b/includes/SkinVersionLookup.php index a32c4f63..3f1c74cb 100644 --- a/includes/SkinVersionLookup.php +++ b/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 ) { diff --git a/tests/phpunit/integration/SkinVersionLookupTest.php b/tests/phpunit/integration/SkinVersionLookupTest.php index b870440e..93eb0dc2 100644 --- a/tests/phpunit/integration/SkinVersionLookupTest.php +++ b/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 */