jonny
/
Vector-Local
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix language alert regression 

- Add test for SkinVector::shouldLanguageAlertBeInSidebar() method.
- Change some methods to protected status -- acknowledging this is not
an optimal change. Until we can pull out some of the language checks
into its own class with injected dependencies (title, feature manager,
languages), this is an interim solution to try to catch regressions
since the conditional states for showing the alert are complex and
ultimately temporary. Extracting the language checks into their own
class can be done in a follow up ticket to help optimize testing
language checks in isolation.

Bug: T302018
Change-Id: I99b7df3029e0af86e4d67b3f446d4ce99260d33e
This commit is contained in:
Clare Ming 2022-02-18 15:44:41 -07:00
parent 9c9d8693c5
commit 338e0e72ac
2 changed files with 191 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
includes/SkinVector.php
View File

@ -186,7 +186,7 @@ class SkinVector extends SkinMustache {
* Calls getLanguages with caching.
* @return array
*/
private function getLanguagesCached(): array {
protected function getLanguagesCached(): array {
if ( $this->languages === null ) {
$this->languages = $this->getLanguages();
}
@ -216,7 +216,7 @@ class SkinVector extends SkinMustache {
* @param string $location Either 'top' or 'bottom' is accepted.
* @return bool
*/
private function isLanguagesInContentAt( $location ) {
protected function isLanguagesInContentAt( $location ) {
if ( !$this->canHaveLanguages() ) {
return false;
}
@ -254,7 +254,7 @@ class SkinVector extends SkinMustache {
* and the language array isn't empty. Hide it otherwise.
* @return bool
*/
private function shouldHideLanguages() {
protected function shouldHideLanguages() {
// NOTE: T276950 - This should be revisited when an empty state for the language button is chosen.
return $this->isLegacy() || !$this->isLanguagesInContent() || empty( $this->getLanguagesCached() );
}
@ -542,7 +542,7 @@ class SkinVector extends SkinMustache {
$isMainPage = $this->getTitle() ? $this->getTitle()->isMainPage() : false;
$shouldShowOnMainPage = $isMainPage && !empty( $this->getLanguagesCached() ) &&
$featureManager->isFeatureEnabled( Constants::FEATURE_LANGUAGE_IN_MAIN_PAGE_HEADER );
return ( $this->isLanguagesInContentAt( 'top' ) && !$isMainPage &&
return ( $this->isLanguagesInContentAt( 'top' ) && !$isMainPage && !$this->shouldHideLanguages() &&
$featureManager->isFeatureEnabled( Constants::FEATURE_LANGUAGE_ALERT_IN_SIDEBAR ) ) ||
$shouldShowOnMainPage;
}

187
tests/phpunit/integration/SkinVectorTest.php
View File

@ -1,7 +1,10 @@
<?php
namespace MediaWiki\Skins\Vector\Tests\Integration;
use Exception;
use HashConfig;
use MediaWikiIntegrationTestCase;
use ReflectionMethod;
use RequestContext;
use SkinVector;
use Title;
@ -233,4 +236,188 @@ class SkinVectorTest extends MediaWikiIntegrationTestCase {
);
}
/**
* Standard config for Language Alert in Sidebar
* @return array
*/
private function enableLanguageAlertFeatureConfig(): array {
return [
'VectorLanguageInHeader' => [
'logged_in' => true,
'logged_out' => true
],
'VectorLanguageInMainPageHeader' => [
'logged_in' => false,
'logged_out' => false
],
'VectorLanguageAlertInSidebar' => [
'logged_in' => true,
'logged_out' => true
],
];
}
public function providerLanguageAlertRequirements() {
$testTitle = Title::makeTitle( NS_MAIN, 'Test' );
$testTitleMainPage = Title::makeTitle( NS_MAIN, 'MAIN_PAGE' );
return [
'When none of the requirements are present, do not show alert' => [
// Configuration requirements for language in header and alert in sidebar
[],
// Title instance
$testTitle,
// Cached languages
[],
// Is the language selector at the top of the content?
false,
// Should the language button be hidden?
false,
// Expected
false
],
'When the feature is enabled and languages should be hidden, do not show alert' => [
$this->enableLanguageAlertFeatureConfig(),
$testTitle,
[], true, true, false
],
'When the language alert feature is disabled, do not show alert' => [
[
'VectorLanguageInHeader' => [
'logged_in' => true,
'logged_out' => true
],
'VectorLanguageAlertInSidebar' => [
'logged_in' => false,
'logged_out' => false
]
],
$testTitle,
[ 'fr', 'en', 'ko' ], true, false, false
],
'When the language in header feature is disabled, do not show alert' => [
[
'VectorLanguageInHeader' => [
'logged_in' => false,
'logged_out' => false
],
'VectorLanguageAlertInSidebar' => [
'logged_in' => true,
'logged_out' => true
]
],
$testTitle,
[ 'fr', 'en', 'ko' ], true, false, false
],
'When it is a main page, feature is enabled, and there are no languages, do not show alert' => [
$this->enableLanguageAlertFeatureConfig(),
$testTitleMainPage,
[], true, true, false
],
'When it is a non-main page, feature is enabled, and there are no languages, do not show alert' => [
$this->enableLanguageAlertFeatureConfig(),
$testTitle,
[], true, true, false
],
'When it is a main page, header feature is disabled, and there are languages, do not show alert' => [
[
'VectorLanguageInHeader' => [
'logged_in' => false,
'logged_out' => false
],
'VectorLanguageAlertInSidebar' => [
'logged_in' => true,
'logged_out' => true
]
],
$testTitleMainPage,
[ 'fr', 'en', 'ko' ], true, true, false
],
'When it is a non-main page, alert feature is disabled, there are languages, do not show alert' => [
[
'VectorLanguageInHeader' => [
'logged_in' => true,
'logged_out' => true
],
'VectorLanguageAlertInSidebar' => [
'logged_in' => false,
'logged_out' => false
]
],
$testTitle,
[ 'fr', 'en', 'ko' ], true, true, false
],
'When most requirements are present but languages are not at the top, do not show alert' => [
$this->enableLanguageAlertFeatureConfig(),
$testTitle,
[ 'fr', 'en', 'ko' ], false, false, false
],
'When most requirements are present but languages should be hidden, do not show alert' => [
$this->enableLanguageAlertFeatureConfig(),
$testTitle,
[ 'fr', 'en', 'ko' ], true, true, false
],
'When it is a main page, features are enabled, and there are languages, show alert' => [
$this->enableLanguageAlertFeatureConfig(),
$testTitleMainPage,
[ 'fr', 'en', 'ko' ], true, false, true
],
'When all the requirements are present on a non-main page, show alert' => [
$this->enableLanguageAlertFeatureConfig(),
$testTitle,
[ 'fr', 'en', 'ko' ], true, false, true
],
];
}
/**
* @dataProvider providerLanguageAlertRequirements
* @covers ::shouldLanguageAlertBeInSidebar
* @param array $requirements
* @param Title $title
* @param array $getLanguagesCached
* @param bool $isLanguagesInContentAt
* @param bool $shouldHideLanguages
* @param bool $expected
* @throws Exception
*/
public function testShouldLanguageAlertBeInSidebar(
array $requirements,
Title $title,
array $getLanguagesCached,
bool $isLanguagesInContentAt,
bool $shouldHideLanguages,
bool $expected
) {
$config = new HashConfig( array_merge( $requirements, [
'DefaultSkin' => 'vector-2022',
'VectorDefaultSkinVersion' => '2',
'VectorSkinMigrationMode' => true,
] ) );
$this->installMockMwServices( $config );
$mockSkinVector = $this->getMockBuilder( SkinVector::class )
->disableOriginalConstructor()
->onlyMethods( [ 'getTitle', 'getLanguagesCached','isLanguagesInContentAt', 'shouldHideLanguages' ] )
->getMock();
$mockSkinVector->method( 'getTitle' )
->willReturn( $title );
$mockSkinVector->method( 'getLanguagesCached' )
->willReturn( $getLanguagesCached );
$mockSkinVector->method( 'isLanguagesInContentAt' )->with( 'top' )
->willReturn( $isLanguagesInContentAt );
$mockSkinVector->method( 'shouldHideLanguages' )
->willReturn( $shouldHideLanguages );
$shouldLanguageAlertBeInSidebarMethod = new ReflectionMethod(
SkinVector::class,
'shouldLanguageAlertBeInSidebar'
);
$shouldLanguageAlertBeInSidebarMethod->setAccessible( true );
$this->assertSame(
$shouldLanguageAlertBeInSidebarMethod->invoke( $mockSkinVector ),
$expected
);
}
}