From 435c903523f054b3d07ca643cd4adc15dc59b4f0 Mon Sep 17 00:00:00 2001
From: Jon Robson <jdlrobson@gmail.com>
Date: Mon, 24 Jan 2022 12:11:52 -0800
Subject: [PATCH] Fix bug in SkinVersionLookup

Bug: T299971
Change-Id: Icd8874315bf3c5846b00e8c34eb1a739c4a0feba
---
 includes/SkinVersionLookup.php                | 10 ++++--
 .../integration/SkinVersionLookupTest.php     | 36 ++++++++++++++-----
 2 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/includes/SkinVersionLookup.php b/includes/SkinVersionLookup.php
index 3f1c74cbb..6ce90a14a 100644
--- a/includes/SkinVersionLookup.php
+++ b/includes/SkinVersionLookup.php
@@ -108,17 +108,21 @@ final class SkinVersionLookup {
 	 */
 	public function getVersion(): string {
 		$migrationMode = $this->config->get( 'VectorSkinMigrationMode' );
+		$useSkin = $this->request->getVal(
+			Constants::QUERY_PARAM_SKIN
+		);
 		// 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;
 			}
 		}
+		// [[phab:T299971]]
+		if ( $useSkin === Constants::SKIN_NAME_MODERN ) {
+			return Constants::SKIN_VERSION_LATEST;
+		}
 
 		// If skin key is not vector, then version should be considered legacy.
 
diff --git a/tests/phpunit/integration/SkinVersionLookupTest.php b/tests/phpunit/integration/SkinVersionLookupTest.php
index 93eb0dc26..805189a14 100644
--- a/tests/phpunit/integration/SkinVersionLookupTest.php
+++ b/tests/phpunit/integration/SkinVersionLookupTest.php
@@ -36,8 +36,13 @@ class SkinVersionLookupTest extends \MediaWikiIntegrationTestCase {
 		$request = $this->getMockBuilder( \WebRequest::class )->getMock();
 		$request
 			->method( 'getVal' )
-			->with( $this->anything(), 'beta' )
-			->willReturn( 'alpha' );
+			->willReturnCallback( static function ( $key ) {
+				if ( $key === Constants::QUERY_PARAM_SKIN ) {
+					return null;
+				} else {
+					return 'alpha';
+				}
+			} );
 
 		$user = $this->createMock( \User::class );
 		$user
@@ -76,8 +81,13 @@ class SkinVersionLookupTest extends \MediaWikiIntegrationTestCase {
 		$request = $this->getMockBuilder( \WebRequest::class )->getMock();
 		$request
 			->method( 'getVal' )
-			->with( $this->anything(), 'beta' )
-			->willReturn( 'beta' );
+			->willReturnCallback( static function ( $key ) {
+				if ( $key === Constants::QUERY_PARAM_SKIN ) {
+					return null;
+				} else {
+					return 'beta';
+				}
+			} );
 
 		$user = $this->createMock( \User::class );
 		$user
@@ -116,8 +126,13 @@ class SkinVersionLookupTest extends \MediaWikiIntegrationTestCase {
 		$request = $this->getMockBuilder( \WebRequest::class )->getMock();
 		$request
 			->method( 'getVal' )
-			->with( $this->anything(), '1' )
-			->willReturn( '1' );
+			->willReturnCallback( static function ( $key ) {
+				if ( $key === Constants::QUERY_PARAM_SKIN ) {
+					return null;
+				} else {
+					return '1';
+				}
+			} );
 
 		$user = $this->createMock( \User::class );
 		$user
@@ -296,8 +311,13 @@ class SkinVersionLookupTest extends \MediaWikiIntegrationTestCase {
 		$request = $this->getMockBuilder( \WebRequest::class )->getMock();
 		$request
 			->method( 'getVal' )
-			->with( $this->anything(), '2' )
-			->willReturn( '2' );
+			->willReturnCallback( static function ( $key ) {
+				if ( $key === Constants::QUERY_PARAM_SKIN ) {
+					return null;
+				} else {
+					return '2';
+				}
+			} );
 
 		$user = $this->createMock( \User::class );
 		$user