Bmansurov has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/353481 )
Change subject: Check data type before calling member function
......................................................................
Check data type before calling member function
The functions MWEchoNotifUser::getLastUnreadNotificationTime() and
EchoSeenTime::getTime() may return ‘false’. The patch is a fix for
issues that may arise from using the results of these functions
without checking the types.
While we’re at it, add tests for SkinMinerva::prepareUserButton() that
cover bugs described above.
Bug: T161022
Change-Id: I87205a5ee4d451b0eb10b41b01c315661763b405
---
M includes/skins/SkinMinerva.php
M tests/phpunit/skins/SkinMinervaTest.php
2 files changed, 215 insertions(+), 3 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend
refs/changes/81/353481/1
diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php
index 1b52bff..eb17415 100644
--- a/includes/skins/SkinMinerva.php
+++ b/includes/skins/SkinMinerva.php
@@ -383,6 +383,33 @@
}
/**
+ * Get Echo notification target user
+ * @param User $user
+ * @return MWEchoNotifUser
+ */
+ protected function getEchoNotifUser( User $user ) {
+ return MWEchoNotifUser::newFromUser( $user );
+ }
+
+ /**
+ * Get the last time user has seen Echo notifications
+ * @param User $user
+ * @return string|bool Timestamp in TS_ISO_8601 format, or false if no
stored time
+ */
+ protected function getEchoSeenTime( User $user ) {
+ return EchoSeenTime::newFromUser( $user )->getTime( 'all',
/*flags*/ 0, TS_ISO_8601 );
+ }
+
+ /**
+ * Get formatted Echo notification count
+ * @param int $count
+ * @param string
+ */
+ protected function getFormattedEchoNotificationCount( int $count ) {
+ return EchoNotificationController::formatNotificationCount(
$count );
+ }
+
+ /**
* Prepares the user button.
* @param QuickTemplate $tpl
*/
@@ -406,8 +433,8 @@
// Don't show the secondary button at all
$notificationsTitle = null;
} else {
- $notifUser = MWEchoNotifUser::newFromUser(
$user );
- $echoSeenTime = EchoSeenTime::newFromUser(
$user )->getTime( 'all', /*flags*/ 0, TS_ISO_8601 );
+ $notifUser = $this->getEchoNotifUser( $user );
+ $echoSeenTime = $this->getEchoSeenTime( $user );
$notificationsMsg = $this->msg(
'mobile-frontend-user-button-tooltip' )->text();
$notifLastUnreadTime =
$notifUser->getLastUnreadNotificationTime();
@@ -416,10 +443,12 @@
$isZero = $count === 0;
$hasUnseen = (
$count > 0 &&
+ $echoSeenTime !== false &&
+ $notifLastUnreadTime !== false &&
$echoSeenTime <
$notifLastUnreadTime->getTimestamp( TS_ISO_8601 )
);
- $countLabel =
EchoNotificationController::formatNotificationCount( $count );
+ $countLabel =
$this->getFormattedEchoNotificationCount( $count );
}
} elseif ( !empty( $newtalks ) ) {
$notificationsTitle = SpecialPage::getTitleFor(
'Mytalk' );
diff --git a/tests/phpunit/skins/SkinMinervaTest.php
b/tests/phpunit/skins/SkinMinervaTest.php
index a0a29af..f5d5157 100644
--- a/tests/phpunit/skins/SkinMinervaTest.php
+++ b/tests/phpunit/skins/SkinMinervaTest.php
@@ -3,11 +3,34 @@
namespace Tests\MobileFrontend\Skins;
use MediaWikiTestCase;
+use MobileUI;
+use MWTimestamp;
use OutputPage;
+use QuickTemplate;
use RequestContext;
use SkinMinerva;
+use SpecialPage;
use Title;
+use User;
use Wikimedia\TestingAccessWrapper;
+
+class Template extends QuickTemplate {
+ public function execute() {
+ }
+}
+
+class EchoNotifUser {
+ public function __construct( $echoLastUnreadNotificationTime,
$echoNotificationCount ) {
+ $this->echoLastUnreadNotificationTime =
$echoLastUnreadNotificationTime;
+ $this->echoNotificationCount = $echoNotificationCount;
+ }
+ public function getLastUnreadNotificationTime() {
+ return $this->echoLastUnreadNotificationTime;
+ }
+ public function getNotificationCount() {
+ return $this->echoNotificationCount;
+ }
+}
/**
* @covers SkinMinerva
@@ -174,4 +197,164 @@
[ false, false, 'skins.minerva.backtotop', false ],
];
}
+
+ /**
+ * Test the notification user button
+ *
+ * @covers SkinMinerva:prepareUserButton
+ * @dataProvider providePrepareUserButton
+ * @param array|string $expectedSecondaryButtonData Expected test case
outcome
+ * @param string $message Test message
+ * @param Title $title
+ * @param bool $useEcho Whether to use Extension:Echo
+ * @param bool $isUserLoggedIn
+ * @param string $newtalks New talk page messages for the current user
+ * @param MWTimestamp|bool $echoLastUnreadNotificationTime Timestamp or
false
+ * @param int $echoNotificationCount
+ * @param string|bool $echoSeenTime String in format TS_ISO_8601 or
false
+ * @param string $formattedEchoNotificationCount
+ */
+ public function testPrepareUserButton(
+ $expectedSecondaryButtonData, $message, $title, $useEcho,
$isUserLoggedIn,
+ $newtalks, $echoLastUnreadNotificationTime = false,
+ $echoNotificationCount = false, $echoSeenTime = false,
+ $formattedEchoNotificationCount = false
+ ) {
+ $user = $this->getMockBuilder( User::class )
+ ->disableOriginalConstructor()
+ ->setMethods( [ 'isLoggedIn' ] )
+ ->getMock();
+ $user->expects( $this->any() )
+ ->method( 'isLoggedIn' )
+ ->will( $this->returnValue( $isUserLoggedIn ) );
+
+ $skin = TestingAccessWrapper::newFromObject(
+ $this->getMockBuilder( SkinMinerva::class )
+ ->disableOriginalConstructor()
+ ->setMethods( [ 'getTitle', 'getUser',
'getNewtalks', 'useEcho',
+
'getEchoNotifUser', 'getEchoSeenTime',
+
'getFormattedEchoNotificationCount' ] )
+ ->getMock()
+ );
+ $skin->expects( $this->any() )
+ ->method( 'getTitle' )
+ ->will( $this->returnValue( $title ) );
+ $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 ) );
+ $skin->expects( $this->any() )
+ ->method( 'getEchoNotifUser' )
+ ->will( $this->returnValue(
+ new EchoNotifUser(
+ $echoLastUnreadNotificationTime,
$echoNotificationCount
+ )
+ ) );
+ $skin->expects( $this->any() )
+ ->method( 'getEchoSeenTime' )
+ ->will( $this->returnValue( $echoSeenTime ) );
+ $skin->expects( $this->any() )
+ ->method( 'getFormattedEchoNotificationCount' )
+ ->will( $this->returnValue(
$formattedEchoNotificationCount ) );
+
+ $tpl = new Template();
+ $skin->prepareUserButton( $tpl );
+ $this->assertEquals(
+ $expectedSecondaryButtonData,
+ $tpl->get( 'secondaryButtonData' ),
+ $message
+ );
+ }
+
+ public function providePrepareUserButton() {
+ function getExpectedResult( $title, $notificationsMsg,
$notificationsTitle,
+
$countLabel, $isZero, $hasUnseen
+ ) {
+ return [
+ 'class' => MobileUI::iconClass( 'notifications'
),
+ 'title' => $notificationsMsg,
+ 'url' => SpecialPage::getTitleFor(
$notificationsTitle )
+ ->getLocalURL(
+ [ 'returnto' =>
$title->getPrefixedText() ] ),
+ 'notificationCount' => $countLabel,
+ 'isNotificationCountZero' => $isZero,
+ 'hasUnseenNotifications' => $hasUnseen
+ ];
+ }
+
+ $title = Title::newFromText( 'Test' );
+ return [
+ [ '', 'No Echo, not logged in, no talk page alerts',
+ $title, false, false, '' ],
+ [ '', 'No Echo, logged in, no talk page alerts',
+ $title, false, true, '' ],
+ [ '', 'Echo, not logged in, no talk page alerts',
+ $title, true, false, '' ],
+ [ getExpectedResult(
+ $title,
+ 'You have new messages on your talk page',
+ 'Mytalk',
+ '',
+ true,
+ false
+ ), 'No Echo, not logged in, talk page alert',
+ $title, false, false, 'newtalks alert' ],
+ [ '', 'Echo, logged in, no talk page alerts',
+ Title::newFromText( 'Special:Notifications' ), true,
true, '' ],
+ [ '', 'Echo, logged in, talk page alert',
+ Title::newFromText( 'Special:Notifications' ), true,
true,
+ 'newtalks alert' ],
+ [ getExpectedResult(
+ $title,
+ 'Show my notifications',
+ 'Notifications',
+ '99+',
+ false,
+ true
+ ), 'Echo, logged in, no talk page alerts, 110
notifications, ' +
+ 'last un-read nofication time after last echo seen
time',
+ $title, true, true, '',
+ MWTimestamp::getInstance( 1494537800 /*
'2017-05-11T21:23:20Z' */ ),
+ 110, '2017-05-11T20:23:20Z', '99+' ],
+ [ getExpectedResult(
+ $title,
+ 'Show my notifications',
+ 'Notifications',
+ '3',
+ false,
+ false
+ ), 'Echo, logged in, no talk page alerts, 3
notifications, ' +
+ 'last un-read nofication time before last echo seen
time',
+ $title, true, true, '',
+ MWTimestamp::getInstance( 1494537800 /*
'2017-05-11T21:23:20Z' */ ),
+ 3, '2017-05-11T22:23:20Z', '3' ],
+ [ getExpectedResult(
+ $title,
+ 'Show my notifications',
+ 'Notifications',
+ '5',
+ false,
+ false
+ ), 'Echo, logged in, no talk page alerts, 5
notifications, ' +
+ 'no last un-read nofication time',
+ $title, true, true, '', false, 5,
'2017-05-11T22:23:20Z', '5' ],
+ [ getExpectedResult(
+ $title,
+ 'Show my notifications',
+ 'Notifications',
+ '0',
+ true,
+ false
+ ), 'Echo, logged in, no talk page alerts, 0
notifications, ' +
+ 'no last echo seen time',
+ $title, true, true, '',
+ MWTimestamp::getInstance( 1494537800 ),
+ 0, false, '0' ]
+ ];
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/353481
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I87205a5ee4d451b0eb10b41b01c315661763b405
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Bmansurov <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
