From d1072d0fdfb15710659d3b928d9a37bdb3496514 Mon Sep 17 00:00:00 2001 From: Stephen Niedzielski Date: Tue, 31 Mar 2020 16:34:50 -0600 Subject: [PATCH] [dev] add skin version query parameter override As described in the readme but not implemented until now, this patch enables the skin version to be specified as a URL query parameter. This is useful for testing both skin versions during development and on wiki, as well as enabling sharing URLs with a specific skin (Vector) and skin version (1 or 2). Obtaining the actual skin version requires tying together three input sources, WebRequest, User, and Config. It seems simple but it'd be easy to botch. For this reason, a helper class to correctly interrogate them and tests are provided. Bug: T244481 Change-Id: I52d80942b4270c008d4e45050589ed9220255a50 --- README.md | 3 +- includes/Constants.php | 7 + includes/SkinVector.php | 35 ++--- includes/SkinVersionLookup.php | 109 ++++++++++++++ skin.json | 1 + .../integration/SkinVersionLookupTest.php | 133 ++++++++++++++++++ 6 files changed, 271 insertions(+), 17 deletions(-) create mode 100644 includes/SkinVersionLookup.php create mode 100644 tests/phpunit/integration/SkinVersionLookupTest.php diff --git a/README.md b/README.md index 7e2b592..bcd6774 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,8 @@ URL query parameters -------------------- - `useskinversion`: Like `useskin` but for overriding the Vector skin version - user preference and configuration. + user preference and configuration. E.g., + http://localhost:8181?useskin=vector&useskinversion=2. Skin preferences ---------------- diff --git a/includes/Constants.php b/includes/Constants.php index e1b0c22..a39359c 100644 --- a/includes/Constants.php +++ b/includes/Constants.php @@ -72,6 +72,13 @@ final class Constants { */ public const REQUIREMENT_FULLY_INITIALISED = 'FullyInitialised'; + // These are used for query parameters. + /** + * Override the skin version user preference and site Config. See readme. + * @var string + */ + public const QUERY_PARAM_SKIN_VERSION = 'useskinversion'; + /** * This class is for namespacing constants only. Forbid construction. * @throws FatalError diff --git a/includes/SkinVector.php b/includes/SkinVector.php index 29a0010..f061826 100644 --- a/includes/SkinVector.php +++ b/includes/SkinVector.php @@ -21,7 +21,7 @@ * @file * @ingroup Skins */ -use Vector\Constants; +use Vector\SkinVersionLookup; /** * Skin subclass for Vector @@ -36,6 +36,21 @@ class SkinVector extends SkinTemplate { private $responsiveMode = false; + /** + * @var SkinVersionLookup + */ + private $skinVersionLookup; + + /** + * @inheritDoc + */ + public function __construct( $skinname = null ) { + parent::__construct( $skinname ); + + $this->skinVersionLookup = + new SkinVersionLookup( $this->getRequest(), $this->getUser(), $this->getConfig() ); + } + /** * Enables the responsive mode */ @@ -67,7 +82,8 @@ class SkinVector extends SkinTemplate { public function getDefaultModules() { $modules = parent::getDefaultModules(); // add vector skin styles and vector module - $module = $this->isLegacy() ? 'skins.vector.styles.legacy' : 'skins.vector.styles'; + $module = $this->skinVersionLookup->isLegacy() + ? 'skins.vector.styles.legacy' : 'skins.vector.styles'; $modules['styles']['skin'][] = $module; $modules['core'][] = 'skins.vector.js'; @@ -85,9 +101,7 @@ class SkinVector extends SkinTemplate { */ protected function setupTemplate( $classname ) { $tp = new TemplateParser( __DIR__ . '/templates' ); - $vectorTemplate = new VectorTemplate( $this->getConfig(), $tp, $this->isLegacy() ); - - return $vectorTemplate; + return new VectorTemplate( $this->getConfig(), $tp, $this->skinVersionLookup->isLegacy() ); } /** @@ -98,15 +112,4 @@ class SkinVector extends SkinTemplate { public function shouldPreloadLogo() { return true; } - - /** - * Whether or not the legacy skin is being used. - * - * @return bool - */ - private function isLegacy() { - // Note: This will be replaced with FeatureManager when it is ready. - return $this->getUser()->getOption( Constants::PREF_KEY_SKIN_VERSION ) - === Constants::SKIN_VERSION_LEGACY; - } } diff --git a/includes/SkinVersionLookup.php b/includes/SkinVersionLookup.php new file mode 100644 index 0000000..8b43a3f --- /dev/null +++ b/includes/SkinVersionLookup.php @@ -0,0 +1,109 @@ +request = $request; + $this->user = $user; + $this->config = $config; + } + + /** + * Whether or not the legacy skin is being used. + * + * @return bool + * @throws \ConfigException + */ + public function isLegacy() { + return $this->getVersion() === Constants::SKIN_VERSION_LEGACY; + } + + /** + * The skin version as a string. E.g., `Constants::SKIN_VERSION_LEGACY`, + * `Constants::SKIN_VERSION_LATEST`, or maybe 'beta'. Note: it's likely someone will put arbitrary + * strings in the query parameter which means this function returns those strings as is. + * + * @return string + * @throws \ConfigException + */ + public function getVersion() { + // Obtain the skin version from the `useskinversion` URL query parameter override, the user + // preference, or the configured default. + return (string)$this->request->getVal( + Constants::QUERY_PARAM_SKIN_VERSION, + $this->user->getOption( + Constants::PREF_KEY_SKIN_VERSION, + $this->config->get( Constants::CONFIG_KEY_DEFAULT_SKIN_VERSION ) + ) + ); + } +} diff --git a/skin.json b/skin.json index c058412..f4620b9 100644 --- a/skin.json +++ b/skin.json @@ -24,6 +24,7 @@ "AutoloadClasses": { "Vector\\Constants": "includes/Constants.php", "Vector\\Hooks": "includes/Hooks.php", + "Vector\\SkinVersionLookup": "includes/SkinVersionLookup.php", "SkinVector": "includes/SkinVector.php", "VectorTemplate": "includes/VectorTemplate.php" }, diff --git a/tests/phpunit/integration/SkinVersionLookupTest.php b/tests/phpunit/integration/SkinVersionLookupTest.php new file mode 100644 index 0000000..59fa77a --- /dev/null +++ b/tests/phpunit/integration/SkinVersionLookupTest.php @@ -0,0 +1,133 @@ +getMockBuilder( \WebRequest::class )->getMock(); + $request + ->expects( $this->exactly( 2 ) ) + ->method( 'getVal' ) + ->with( $this->anything(), $this->equalTo( '1' ) ) + ->willReturn( 'beta' ); + + $user = $this->createMock( \User::class ); + $user + ->expects( $this->exactly( 2 ) ) + ->method( 'getOption' ) + ->with( $this->anything(), $this->equalTo( '2' ) ) + ->willReturn( '1' ); + + $config = new HashConfig( [ 'VectorDefaultSkinVersion' => '2' ] ); + + $skinVersionLookup = new SkinVersionLookup( $request, $user, $config ); + + $this->assertSame( + $skinVersionLookup->getVersion(), + 'beta', + 'Query parameter is the first priority.' + ); + $this->assertSame( + $skinVersionLookup->isLegacy(), + false, + 'Version is non-legacy.' + ); + } + + /** + * @covers ::getVersion + * @covers ::isLegacy + */ + public function testUserPreference() { + $request = $this->getMockBuilder( \WebRequest::class )->getMock(); + $request + ->expects( $this->exactly( 2 ) ) + ->method( 'getVal' ) + ->with( $this->anything(), $this->equalTo( '1' ) ) + ->willReturn( '1' ); + + $user = $this->createMock( \User::class ); + $user + ->expects( $this->exactly( 2 ) ) + ->method( 'getOption' ) + ->with( $this->anything(), $this->equalTo( '2' ) ) + ->willReturn( '1' ); + + $config = new HashConfig( [ 'VectorDefaultSkinVersion' => '2' ] ); + + $skinVersionLookup = new SkinVersionLookup( $request, $user, $config ); + + $this->assertSame( + $skinVersionLookup->getVersion(), + '1', + 'User preference is the second priority.' + ); + $this->assertSame( + $skinVersionLookup->isLegacy(), + true, + 'Version is legacy.' + ); + } + + /** + * @covers ::getVersion + * @covers ::isLegacy + */ + public function testConfig() { + $request = $this->getMockBuilder( \WebRequest::class )->getMock(); + $request + ->expects( $this->exactly( 2 ) ) + ->method( 'getVal' ) + ->with( $this->anything(), $this->equalTo( '2' ) ) + ->willReturn( '2' ); + + $user = $this->createMock( \User::class ); + $user + ->expects( $this->exactly( 2 ) ) + ->method( 'getOption' ) + ->with( $this->anything(), $this->equalTo( '2' ) ) + ->willReturn( '2' ); + + $config = new HashConfig( [ 'VectorDefaultSkinVersion' => '2' ] ); + + $skinVersionLookup = new SkinVersionLookup( $request, $user, $config ); + + $this->assertSame( + $skinVersionLookup->getVersion(), + '2', + 'Config is the third priority.' + ); + $this->assertSame( + $skinVersionLookup->isLegacy(), + false, + 'Version is non-legacy.' + ); + } +}