From 39a207847258f8db1fedee9667f2be2dcd9ed62c Mon Sep 17 00:00:00 2001 From: Piotr Miazga Date: Tue, 7 May 2019 22:27:56 +0200 Subject: [PATCH] Minerva shouldn't call Skin::getNewTalks() twice The Skin::getNewTalks() is already called in SkinTemplate::prepareQuickTemplate. There is no need to call this function again, as is pretty heavy. Instead of calling it second time, just re-use the value stored in the QuickTemplate Change-Id: I0e9491405f4d760278db3a423ee14e8f80720291 --- includes/skins/SkinMinerva.php | 8 ++++---- tests/phpunit/skins/SkinMinervaTest.php | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php index 32f195d..1ff0ca5 100644 --- a/includes/skins/SkinMinerva.php +++ b/includes/skins/SkinMinerva.php @@ -149,7 +149,7 @@ class SkinMinerva extends SkinTemplate { $this->prepareMenuButton( $tpl ); $this->prepareBanners( $tpl ); $this->preparePageActions( $tpl ); - $this->prepareUserButton( $tpl ); + $this->prepareUserButton( $tpl, $tpl->get( 'newtalk' ) ); $this->prepareLanguages( $tpl ); return $tpl; @@ -385,8 +385,9 @@ class SkinMinerva extends SkinTemplate { /** * Prepares the user button. * @param QuickTemplate $tpl + * @param string $newTalks New talk page messages for the current user */ - protected function prepareUserButton( QuickTemplate $tpl ) { + protected function prepareUserButton( QuickTemplate $tpl, $newTalks ) { // Set user button to empty string by default $tpl->set( 'secondaryButtonData', '' ); $notificationsTitle = ''; @@ -396,7 +397,6 @@ class SkinMinerva extends SkinTemplate { $hasUnseen = false; $user = $this->getUser(); - $newtalks = $this->getNewtalks(); $currentTitle = $this->getTitle(); // If Echo is available, the user is logged in, and they are not already on the @@ -431,7 +431,7 @@ class SkinMinerva extends SkinTemplate { $countLabel = $this->getFormattedEchoNotificationCount( $count ); } - } elseif ( !empty( $newtalks ) ) { + } elseif ( !empty( $newTalks ) ) { $notificationsTitle = SpecialPage::getTitleFor( 'Mytalk' ); $notificationsMsg = $this->msg( 'mobile-frontend-user-newmessages' )->text(); } diff --git a/tests/phpunit/skins/SkinMinervaTest.php b/tests/phpunit/skins/SkinMinervaTest.php index f8e241a..6edf328 100644 --- a/tests/phpunit/skins/SkinMinervaTest.php +++ b/tests/phpunit/skins/SkinMinervaTest.php @@ -2,6 +2,7 @@ namespace Tests\MediaWiki\Minerva; +use MediaWiki\MediaWikiServices; use MediaWiki\Minerva\SkinOptions; use MediaWikiTestCase; use MinervaUI; @@ -202,7 +203,7 @@ class SkinMinervaTest extends MediaWikiTestCase { $skin = TestingAccessWrapper::newFromObject( $this->getMockBuilder( SkinMinerva::class ) ->disableOriginalConstructor() - ->setMethods( [ 'getTitle', 'getUser', 'getNewtalks', 'useEcho', + ->setMethods( [ 'getTitle', 'getUser', 'useEcho', 'getEchoNotifUser', 'getEchoSeenTime', 'getFormattedEchoNotificationCount' ] ) ->getMock() @@ -213,9 +214,6 @@ class SkinMinervaTest extends MediaWikiTestCase { $skin->expects( $this->any() ) ->method( 'getUser' ) ->will( $this->returnValue( $user ) ); - $skin->expects( $this->any() ) - ->method( 'getNewtalks' ) - ->will( $this->returnValue( $newtalks ) ); $skin->expects( $this->any() ) ->method( 'useEcho' ) ->will( $this->returnValue( $useEcho ) ); @@ -238,8 +236,10 @@ class SkinMinervaTest extends MediaWikiTestCase { $tpl = $this->getMockBuilder( QuickTemplate::class ) ->setMethods( [ 'execute' ] ) + ->setConstructorArgs( [ MediaWikiServices::getInstance()->getMainConfig() ] ) ->getMock(); - $skin->prepareUserButton( $tpl ); + $skin->prepareUserButton( $tpl, $newtalks ); + $this->assertEquals( $expectedSecondaryButtonData, $tpl->get( 'secondaryButtonData' ),