jenkins-bot has submitted this change and it was merged.
Change subject: Add support to mark all as read for "sections"
......................................................................
Add support to mark all as read for "sections"
This also updates the way how mark all as read works
Change-Id: Ifb7b1b7b7feb4a5af65c79bb16b91a5a9c70166c
---
M Echo.php
M api/ApiEchoMarkRead.php
M includes/NotifUser.php
M includes/gateway/UserNotificationGateway.php
M tests/NotifUserTest.php
A tests/api/ApiEchoMarkReadTest.php
6 files changed, 302 insertions(+), 23 deletions(-)
Approvals:
EBernhardson: Looks good to me, approved
jenkins-bot: Verified
diff --git a/Echo.php b/Echo.php
index ac8f765..5e61414 100644
--- a/Echo.php
+++ b/Echo.php
@@ -220,6 +220,12 @@
// The max number showed in bundled message, eg, <user> and 99+ others <action>
$wgEchoMaxNotificationCount = 99;
+// The max number allowed to be updated on a web request, when we mark all
notification
+// as read, it's a bad idea to update on a web request if the number is
incredibly
+// huge, to prevent this, we just fetch 2000 thousand records and mark them as
read.
+// This would cover most of the use cases.
+$wgEchoMaxUpdateCount = 2000;
+
// The time interval between each bundle email in seconds
// set a small number for test wikis, should set this to 0 to disable email
bundling
// if there is no delay queue support
diff --git a/api/ApiEchoMarkRead.php b/api/ApiEchoMarkRead.php
index 8cbb61c..9cde1c2 100644
--- a/api/ApiEchoMarkRead.php
+++ b/api/ApiEchoMarkRead.php
@@ -20,8 +20,12 @@
if ( count( $params['list'] ) ) {
// Make sure there is a limit to the update
$notifUser->markRead( array_slice(
$params['list'], 0, ApiBase::LIMIT_SML2 ) );
+ // Mark all as read
} elseif ( $params['all'] ) {
$notifUser->markAllRead();
+ // Mark all as read for sections
+ } elseif ( $params['sections'] ) {
+ $notifUser->markAllRead( $params['sections'] );
}
}
@@ -54,6 +58,10 @@
ApiBase::PARAM_REQUIRED => false,
ApiBase::PARAM_TYPE => 'boolean'
),
+ 'sections' => array(
+ ApiBase::PARAM_TYPE =>
EchoAttributeManager::$sections,
+ ApiBase::PARAM_ISMULTI => true,
+ ),
'token' => array(
ApiBase::PARAM_REQUIRED => true,
),
@@ -65,6 +73,7 @@
return array(
'list' => 'A list of notification IDs to mark as read',
'all' => "If set to true, marks all of a user's
notifications as read",
+ 'sections' => 'A list of sections to mark as read',
'token' => 'edit token',
'uselang' => 'the desired language to format the output'
);
diff --git a/includes/NotifUser.php b/includes/NotifUser.php
index 6a1a83a..0c271fc 100644
--- a/includes/NotifUser.php
+++ b/includes/NotifUser.php
@@ -24,6 +24,12 @@
private $userNotifGateway;
/**
+ * Notification mapper
+ * @var EchoNotificationMapper
+ */
+ private $notifMapper;
+
+ /**
* Whether to check cache for section status
*/
static $sectionStatusCheckCache = array (
@@ -32,15 +38,23 @@
);
/**
- * Constructor for initialization
+ * Usually client code doesn't need to initialize the object directly
+ * because it could be obtained from factory method newFromUser()
* @param User
* @param BagOStuff
* @param EchoUserNotificationGateway
+ * @param EchoNotificationMapper
*/
- private function __construct( User $user, BagOStuff $cache,
EchoUserNotificationGateway $userNotifGateway ) {
+ public function __construct(
+ User $user,
+ BagOStuff $cache,
+ EchoUserNotificationGateway $userNotifGateway,
+ EchoNotificationMapper $notifMapper
+ ) {
$this->mUser = $user;
$this->userNotifGateway = $userNotifGateway;
$this->cache = $cache;
+ $this->notifMapper = $notifMapper;
}
/**
@@ -56,7 +70,8 @@
global $wgMemc;
return new MWEchoNotifUser(
$user, $wgMemc,
- new EchoUserNotificationGateway( $user,
MWEchoDbFactory::newFromDefault() )
+ new EchoUserNotificationGateway( $user,
MWEchoDbFactory::newFromDefault() ),
+ new EchoNotificationMapper()
);
}
@@ -241,32 +256,63 @@
/**
* Mark one or more notifications read for a user.
* @param $eventIds Array of event IDs to mark read
+ * @return boolean
*/
public function markRead( $eventIds ) {
$eventIds = array_filter( (array)$eventIds, 'is_numeric' );
if ( !$eventIds || wfReadOnly() ) {
- return;
- }
-
- $this->userNotifGateway->markRead( $eventIds );
- $this->resetNotificationCount( DB_MASTER );
- }
-
- /**
- * Attempt to mark all notifications as read
- * @return boolean
- */
- public function markAllRead() {
- if ( wfReadOnly() || $this->notifCountHasReachedMax() ) {
return false;
}
- // Only update all the unread notifications if it isn't a huge
number.
- // TODO: Implement batched jobs it's over the maximum.
- $this->userNotifGateway->markAllRead();
- $this->resetNotificationCount( DB_MASTER );
- $this->flagCacheWithNoTalkNotification();
- return true;
+ $res = $this->userNotifGateway->markRead( $eventIds );
+ if ( $res ) {
+ $this->resetNotificationCount( DB_MASTER );
+ }
+ return $res;
+ }
+
+ /**
+ * Attempt to mark all or sections of notifications as read, this only
+ * updates up to $wgEchoMaxUpdateCount records per request, see more
+ * detail about this in Echo.php, the other reason is that mediawiki
+ * database interface doesn't support updateJoin() that would update
+ * across multiple tables, we would visit this later
+ *
+ * @param string[] $sections
+ * @return boolean
+ */
+ public function markAllRead( array $sections = array(
EchoAttributeManager::ALL ) ) {
+ if ( wfReadOnly() ) {
+ return false;
+ }
+
+ global $wgEchoMaxUpdateCount;
+
+ // Mark all sections as read if this is the case
+ if ( in_array( EchoAttributeManager::ALL, $sections ) ) {
+ $sections = EchoAttributeManager::$sections;
+ }
+
+ $attributeManager = EchoAttributeManager::newFromGlobalVars();
+ $eventTypes =
$attributeManager->getUserEnabledEventsbySections( $this->mUser, 'web',
$sections );
+
+ $notifs = $this->notifMapper->fetchUnreadByUser( $this->mUser,
$wgEchoMaxUpdateCount, $eventTypes );
+ $res = $this->markRead( array_map(
+ function( EchoNotification $notif ) {
+ // This should not happen at all, but just 0 in
+ // such case so to keep the code running
+ if ( $notif->getEvent() ) {
+ return $notif->getEvent()->getId();
+ } else {
+ return 0;
+ }
+ },
+ $notifs
+ ) );
+ if ( $res && count( $notifs ) < $wgEchoMaxUpdateCount ) {
+ $this->flagCacheWithNoTalkNotification();
+ }
+ return $res;
}
diff --git a/includes/gateway/UserNotificationGateway.php
b/includes/gateway/UserNotificationGateway.php
index 1a1ac17..5251b77 100644
--- a/includes/gateway/UserNotificationGateway.php
+++ b/includes/gateway/UserNotificationGateway.php
@@ -35,6 +35,7 @@
/**
* Mark notifications as read
* @param $eventIDs array
+ * @return boolean
*/
public function markRead( array $eventIDs ) {
if ( !$eventIDs ) {
@@ -56,7 +57,9 @@
}
/**
- * Mark all notification as read
+ * Mark all notification as read, use MWEchoNotifUer::markAllRead()
instead
+ * @deprecated may need this when running in a job or revive this when
we
+ * have updateJoin()
*/
public function markAllRead() {
$dbw = $this->dbFactory->getEchoDb( DB_MASTER );
diff --git a/tests/NotifUserTest.php b/tests/NotifUserTest.php
index a6216ae..a43a87a 100644
--- a/tests/NotifUserTest.php
+++ b/tests/NotifUserTest.php
@@ -88,4 +88,107 @@
$this->setMwGlobals( 'wgAllowHTMLEmail', $format );
}
+ public function testMarkRead() {
+ global $wgMemc;
+ $notifUser = new MWEchoNotifUser(
+ User::newFromId( 2 ),
+ $wgMemc,
+ $this->mockEchoUserNotificationGateway( array(
'markRead' => true ) ),
+ $this->mockEchoNotificationMapper()
+ );
+ $this->assertFalse( $notifUser->markRead( array() ) );
+ $this->assertTrue( $notifUser->markRead( array( 1 ) ) );
+
+ $notifUser = new MWEchoNotifUser(
+ User::newFromId( 2 ),
+ $wgMemc,
+ $this->mockEchoUserNotificationGateway( array(
'markRead' => false ) ),
+ $this->mockEchoNotificationMapper()
+ );
+ $this->assertFalse( $notifUser->markRead( array() ) );
+ $this->assertFalse( $notifUser->markRead( array( 1 ) ) );
+ }
+
+ public function testMarkAllRead() {
+ global $wgMemc;
+
+ // Successful mark as read & non empty fetch
+ $notifUser = new MWEchoNotifUser(
+ User::newFromId( 2 ),
+ $wgMemc,
+ $this->mockEchoUserNotificationGateway( array(
'markRead' => true ) ),
+ $this->mockEchoNotificationMapper( array(
$this->mockEchoNotification() ) )
+ );
+ $this->assertTrue( $notifUser->markAllRead() );
+
+ // Unsuccessful mark as read & non empty fetch
+ $notifUser = new MWEchoNotifUser(
+ User::newFromId( 2 ),
+ $wgMemc,
+ $this->mockEchoUserNotificationGateway( array(
'markRead' => false ) ),
+ $this->mockEchoNotificationMapper( array(
$this->mockEchoNotification() ) )
+ );
+ $this->assertFalse( $notifUser->markAllRead() );
+
+ // Successful mark as read & empty fetch
+ $notifUser = new MWEchoNotifUser(
+ User::newFromId( 2 ),
+ $wgMemc,
+ $this->mockEchoUserNotificationGateway( array(
'markRead' => true ) ),
+ $this->mockEchoNotificationMapper()
+ );
+ $this->assertFalse( $notifUser->markAllRead() );
+
+ // Unsuccessful mark as read & empty fetch
+ $notifUser = new MWEchoNotifUser(
+ User::newFromId( 2 ),
+ $wgMemc,
+ $this->mockEchoUserNotificationGateway( array(
'markRead' => false ) ),
+ $this->mockEchoNotificationMapper()
+ );
+ $this->assertFalse( $notifUser->markAllRead() );
+ }
+
+ public function mockEchoUserNotificationGateway( array $dbResult =
array() ) {
+ $dbResult += array(
+ 'markRead' => true
+ );
+ $gateway = $this->getMockBuilder( 'EchoUserNotificationGateway'
)
+ ->disableOriginalConstructor()
+ ->getMock();
+ $gateway->expects( $this->any() )
+ ->method( 'markRead' )
+ ->will( $this->returnValue( $dbResult['markRead'] ) );
+ return $gateway;
+ }
+
+ public function mockEchoNotificationMapper( array $result = array() ) {
+ $mapper = $this->getMockBuilder( 'EchoNotificationMapper' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $mapper->expects( $this->any() )
+ ->method( 'fetchUnreadByUser' )
+ ->will( $this->returnValue( $result ) );
+ return $mapper;
+ }
+
+ protected function mockEchoNotification() {
+ $notification = $this->getMockBuilder( 'EchoNotification' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $notification->expects( $this->any() )
+ ->method( 'getEvent' )
+ ->will( $this->returnValue( $this->mockEchoEvent() ) );
+ return $notification;
+ }
+
+ protected function mockEchoEvent() {
+ $event = $this->getMockBuilder( 'EchoEvent' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $event->expects( $this->any() )
+ ->method( 'getId' )
+ ->will( $this->returnValue( 1 ) );
+ return $event;
+ }
}
diff --git a/tests/api/ApiEchoMarkReadTest.php
b/tests/api/ApiEchoMarkReadTest.php
new file mode 100644
index 0000000..104e0b0
--- /dev/null
+++ b/tests/api/ApiEchoMarkReadTest.php
@@ -0,0 +1,112 @@
+<?php
+
+/**
+ * @group medium
+ * @group API
+ * @covers ApiQuery
+ */
+class ApiEchoMarkReadTest extends ApiTestCase {
+
+ protected function setUp() {
+ parent::setUp();
+ $this->doLogin();
+ }
+
+ function getTokens() {
+ return $this->getTokenList( self::$users['sysop'] );
+ }
+
+ public function testMarkReadWithList() {
+
+ $tokens = $this->getTokens();
+ // Grouping by section
+ $data = $this->doApiRequest( array(
+ 'action' => 'echomarkread',
+ 'notlist' => '121|122|123',
+ 'token' => $tokens['edittoken'] ) );
+
+ $this->assertArrayHasKey( 'query', $data[0] );
+ $this->assertArrayHasKey( 'echomarkread', $data[0]['query'] );
+
+ $result = $data[0]['query']['echomarkread'];
+
+ // General count
+ $this->assertArrayHasKey( 'count', $result );
+ $this->assertArrayHasKey( 'rawcount', $result );
+
+ // Alert
+ $this->assertArrayHasKey( 'alert', $result );
+ $alert = $result['alert'];
+ $this->assertArrayHasKey( 'rawcount', $alert );
+ $this->assertArrayHasKey( 'count', $alert );
+
+ // Message
+ $this->assertArrayHasKey( 'message', $result );
+ $message = $result['message'];
+ $this->assertArrayHasKey( 'rawcount', $message );
+ $this->assertArrayHasKey( 'count', $message );
+ }
+
+ public function testMarkReadWithAll() {
+
+ $tokens = $this->getTokens();
+ // Grouping by section
+ $data = $this->doApiRequest( array(
+ 'action' => 'echomarkread',
+ 'notall' => '1',
+ 'token' => $tokens['edittoken'] ) );
+
+ $this->assertArrayHasKey( 'query', $data[0] );
+ $this->assertArrayHasKey( 'echomarkread', $data[0]['query'] );
+
+ $result = $data[0]['query']['echomarkread'];
+
+ // General count
+ $this->assertArrayHasKey( 'count', $result );
+ $this->assertArrayHasKey( 'rawcount', $result );
+
+ // Alert
+ $this->assertArrayHasKey( 'alert', $result );
+ $alert = $result['alert'];
+ $this->assertArrayHasKey( 'rawcount', $alert );
+ $this->assertArrayHasKey( 'count', $alert );
+
+ // Message
+ $this->assertArrayHasKey( 'message', $result );
+ $message = $result['message'];
+ $this->assertArrayHasKey( 'rawcount', $message );
+ $this->assertArrayHasKey( 'count', $message );
+ }
+
+ public function testMarkReadWithSections() {
+
+ $tokens = $this->getTokens();
+ // Grouping by section
+ $data = $this->doApiRequest( array(
+ 'action' => 'echomarkread',
+ 'sections' => 'alert|message',
+ 'token' => $tokens['edittoken'] ) );
+
+ $this->assertArrayHasKey( 'query', $data[0] );
+ $this->assertArrayHasKey( 'echomarkread', $data[0]['query'] );
+
+ $result = $data[0]['query']['echomarkread'];
+
+ // General count
+ $this->assertArrayHasKey( 'count', $result );
+ $this->assertArrayHasKey( 'rawcount', $result );
+
+ // Alert
+ $this->assertArrayHasKey( 'alert', $result );
+ $alert = $result['alert'];
+ $this->assertArrayHasKey( 'rawcount', $alert );
+ $this->assertArrayHasKey( 'count', $alert );
+
+ // Message
+ $this->assertArrayHasKey( 'message', $result );
+ $message = $result['message'];
+ $this->assertArrayHasKey( 'rawcount', $message );
+ $this->assertArrayHasKey( 'count', $message );
+ }
+
+}
--
To view, visit https://gerrit.wikimedia.org/r/153960
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifb7b1b7b7feb4a5af65c79bb16b91a5a9c70166c
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Bsitu <[email protected]>
Gerrit-Reviewer: EBernhardson <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
