Bsitu has uploaded a new change for review.
https://gerrit.wikimedia.org/r/65349
Change subject: (bug 47912) Visting talk page should mark talk notif as read
......................................................................
(bug 47912) Visting talk page should mark talk notif as read
This needs some more manual testing and adding unit testing
Change-Id: Iadfe3cf7927d5318f89ba17f067000f9399060af
---
M Echo.php
M Hooks.php
M Notifier.php
M api/ApiEchoNotifications.php
M controller/NotificationController.php
M includes/DbEchoBackend.php
M includes/EchoBackend.php
A includes/NotifUser.php
M model/Notification.php
M special/SpecialNotifications.php
A tests/NotifUserTest.php
11 files changed, 339 insertions(+), 104 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Echo
refs/changes/49/65349/1
diff --git a/Echo.php b/Echo.php
index e95d103..8b7512a 100644
--- a/Echo.php
+++ b/Echo.php
@@ -85,6 +85,7 @@
$wgAutoloadClasses['MWEchoBackend'] = $dir . 'includes/EchoBackend.php';
$wgAutoloadClasses['MWDbEchoBackend'] = $dir . 'includes/DbEchoBackend.php';
$wgAutoloadClasses['MWEchoDbFactory'] = $dir . 'includes/EchoDbFactory.php';
+$wgAutoloadClasses['MWEchoNotifUser'] = $dir . 'includes/NotifUser.php';
// Whitelist and Blacklist
$wgAutoloadClasses['EchoContainmentList'] = $dir .
'includes/ContainmentSet.php';
@@ -104,6 +105,7 @@
$wgHooks['UserRights'][] = 'EchoHooks::onUserRights';
$wgHooks['UserLoadOptions'][] = 'EchoHooks::onUserLoadOptions';
$wgHooks['UserSaveOptions'][] = 'EchoHooks::onUserSaveOptions';
+$wgHooks['UserClearNewTalkNotification'][] =
'EchoHooks::onUserClearNewTalkNotification';
// Extension initialization
$wgExtensionFunctions[] = 'EchoHooks::initEchoExtension';
@@ -198,6 +200,7 @@
$wgHooks['EchoGetNotificationTypes'][] = 'EchoHooks::getNotificationTypes';
$wgHooks['EchoGetBundleRules'][] = 'EchoHooks::onEchoGetBundleRules';
$wgHooks['EchoAbortEmailNotification'][] =
'EchoHooks::onEchoAbortEmailNotification';
+$wgHooks['EchoCreateNotificationComplete'][] =
'EchoHooks::onEchoCreateNotificationComplete';
// Hook appropriate events
$wgHooks['ArticleSaveComplete'][] = 'EchoHooks::onArticleSaved';
diff --git a/Hooks.php b/Hooks.php
index 01e2ebe..286b64b 100644
--- a/Hooks.php
+++ b/Hooks.php
@@ -626,7 +626,7 @@
return true;
}
- $notificationCount =
EchoNotificationController::getNotificationCount( $wgUser );
+ $notificationCount = MWEchoNotifUser::newFromUser( $wgUser
)->getNotificationCount();
$text = EchoNotificationController::formatNotificationCount(
$notificationCount );
$url = SpecialPage::getTitleFor( 'Notifications'
)->getLocalURL();
if ( $notificationCount == 0 ) {
@@ -678,7 +678,7 @@
$timestamp = new MWTimestamp( wfTimestampNow() );
if ( ! $user->isAnon() ) {
$vars['wgEchoOverlayConfiguration'] = array(
- 'notification-count' =>
EchoNotificationController::getFormattedNotificationCount( $user ),
+ 'notification-count' =>
MWEchoNotifUser::newFromUser( $user )->getFormattedNotificationCount(),
'max-notification-count' =>
$wgEchoMaxNotificationCount,
);
$vars['wgEchoHelpPage'] = $wgEchoHelpPage;
@@ -769,7 +769,7 @@
// Reset the notification count since it may have changed due
to user
// option changes. This covers both explicit changes in the
preferences
// and changes made through the options API (since both call
this hook).
- EchoNotificationController::resetNotificationCount( $user );
+ MWEchoNotifUser::newFromUser( $user )->resetNotificationCount();
return true;
}
@@ -835,4 +835,38 @@
return true;
}
+
+ /**
+ * Handler for UserClearNewTalkNotification hook.
+ * @see
http://www.mediawiki.org/wiki/Manual:Hooks/UserClearNewTalkNotification
+ * @param $user User whose talk page notification should be marked as
read
+ */
+ public static function onUserClearNewTalkNotification( User $user ) {
+ $notifUser = MWEchoNotifUser::newFromUser( $user );
+ if ( $notifUser ) {
+ $notifUser->clearTalkNotification();
+ }
+ return true;
+ }
+
+ /**
+ * Handler for EchoCreateNotificationComplete hook.
+ * @param $notif EchoNotification
+ */
+ public static function onEchoCreateNotificationComplete(
EchoNotification $notif ) {
+ if ( !$notif->getEvent() || !$notif->getUser() ) {
+ return true;
+ }
+
+ $notifUser = MWEchoNotifUser::newFromUser( $notif->getUser() );
+ if ( !$notifUser ) {
+ return true;
+ }
+
+ if ( $notif->getEvent()->getType() === 'edit-user-talk' ) {
+ $notifUser->cacheTalkNotification(
$notif->getEvent()->getId() );
+ }
+
+ return true;
+ }
}
diff --git a/Notifier.php b/Notifier.php
index c6b45d7..2901bf5 100644
--- a/Notifier.php
+++ b/Notifier.php
@@ -23,7 +23,7 @@
MWEchoEventLogging::logSchemaEcho( $user, $event, 'web' );
- EchoNotificationController::resetNotificationCount( $user,
DB_MASTER );
+ MWEchoNotifUser::newFromUser( $user )->resetNotificationCount(
DB_MASTER );
}
/**
diff --git a/api/ApiEchoNotifications.php b/api/ApiEchoNotifications.php
index de11101..5bc3308 100644
--- a/api/ApiEchoNotifications.php
+++ b/api/ApiEchoNotifications.php
@@ -11,15 +11,17 @@
$this->dieUsage( 'Login is required', 'login-required'
);
}
+ $notifUser = MWEchoNotifUser::newFromUser( $user );
+
$params = $this->extractRequestParams();
// There is no need to trigger markRead if all notifications
are read
- if ( EchoNotificationController::getNotificationCount( $user )
> 0 ) {
+ if ( $notifUser->getNotificationCount() > 0 ) {
if ( count( $params['markread'] ) ) {
// Make sure there is a limit to the update
- EchoNotificationController::markRead( $user,
array_slice( $params['markread'], 0, ApiBase::LIMIT_SML2 ) );
+ $notifUser->markRead( array_slice(
$params['markread'], 0, ApiBase::LIMIT_SML2 ) );
} elseif ( $params['markallread'] ) {
- EchoNotificationController::markAllRead( $user
);
+ $notifUser->markAllRead();
}
}
@@ -40,7 +42,7 @@
}
if ( in_array( 'count', $prop ) ) {
- $result['count'] =
EchoNotificationController::getFormattedNotificationCount( $user );
+ $result['count'] =
$notifUser->getFormattedNotificationCount();
}
if ( in_array( 'index', $prop ) ) {
diff --git a/controller/NotificationController.php
b/controller/NotificationController.php
index d15aca2..1809691 100644
--- a/controller/NotificationController.php
+++ b/controller/NotificationController.php
@@ -5,35 +5,6 @@
static protected $userWhitelist;
/**
- * Retrieves number of unread notifications that a user has, would
return
- * $wgEchoMaxNotificationCount + 1 at most
- *
- * @param $user User object to check notifications for
- * @param $cached bool Set to false to bypass the cache.
- * @param $dbSource string use master or slave database to pull count
- * @return Integer: Number of unread notifications.
- */
- public static function getNotificationCount( $user, $cached = true,
$dbSource = DB_SLAVE ) {
- global $wgMemc, $wgEchoBackend, $wgEchoConfig;
-
- if ( $user->isAnon() ) {
- return 0;
- }
-
- $memcKey = wfMemcKey( 'echo-notification-count',
$user->getId(), $wgEchoConfig['version'] );
-
- if ( $cached && $wgMemc->get( $memcKey ) !== false ) {
- return intval( $wgMemc->get( $memcKey ) );
- }
-
- $count = $wgEchoBackend->getNotificationCount( $user, $dbSource
);
-
- $wgMemc->set( $memcKey, $count, 86400 );
-
- return intval( $count );
- }
-
- /**
* Get the enabled events for a user, which excludes user-dismissed
events
* from the general enabled events
* @param $user User
@@ -136,20 +107,6 @@
}
/**
- * Retrieves formatted number of unread notifications that a user has.
- *
- * @param $user User object to check notifications for
- * @param $cached bool Set to false to bypass the cache.
- * @param $dbSource string use master or slave database to pull count
- * @return String: Number of unread notifications.
- */
- public static function getFormattedNotificationCount( $user, $cached =
true, $dbSource = DB_SLAVE ) {
- return self::formatNotificationCount(
- self::getNotificationCount( $user, $cached,
$dbSource )
- );
- }
-
- /**
* Format the notification count with Language::formatNum(). In
addition, for large count,
* return abbreviated version, e.g. 99+
* @param $count int
@@ -168,57 +125,6 @@
}
return $count;
- }
-
- /**
- * Mark one or more notifications read for a user.
- *
- * @param $user User object to mark items read for.
- * @param $eventIDs Array of event IDs to mark read
- */
- public static function markRead( $user, $eventIDs ) {
- global $wgEchoBackend;
-
- $eventIDs = array_filter( (array)$eventIDs, 'is_numeric' );
- if ( !$eventIDs || wfReadOnly() ) {
- return;
- }
- $wgEchoBackend->markRead( $user, $eventIDs );
- self::resetNotificationCount( $user, DB_MASTER );
- }
-
- /**
- * @param $user User to mark all notifications read for
- * @return boolean
- */
- public static function markAllRead( $user ) {
- global $wgEchoBackend, $wgEchoMaxNotificationCount;
-
- if ( wfReadOnly() ) {
- return false;
- }
-
- $notificationCount = self::getNotificationCount( $user );
- // Only update all the unread notifications if it isn't a huge
number.
- // TODO: Implement batched jobs it's over the maximum.
- if ( $notificationCount <= $wgEchoMaxNotificationCount ) {
- $wgEchoBackend->markAllRead( $user );
- self::resetNotificationCount( $user, DB_MASTER );
- return true;
- } else {
- return false;
- }
- }
-
- /**
- * Recalculates the number of notifications that a user has.
- *
- * @param $user User object
- * @param $dbSource string use master or slave database to pull count
- */
- public static function resetNotificationCount( $user, $dbSource =
DB_SLAVE ) {
- self::getNotificationCount( $user, false, $dbSource );
- $user->invalidateCache();
}
/**
diff --git a/includes/DbEchoBackend.php b/includes/DbEchoBackend.php
index 00f5773..2e37c50 100644
--- a/includes/DbEchoBackend.php
+++ b/includes/DbEchoBackend.php
@@ -299,4 +299,32 @@
return $db->numRows( $res );
}
+ /**
+ * Get the unread notification event for a user, should only call this
function
+ * if the number of unread notification is reasonable
+ * @param $user User
+ * @param $type string
+ */
+ public function getUnreadNotifications( $user, $type ) {
+ $dbr = MWEchoDbFactory::getDB( DB_SLAVE );
+ $res = $dbr->select(
+ array( 'echo_notification', 'echo_event' ),
+ array( 'notification_event' ),
+ array(
+ 'notification_user' => $user->getId(),
+ 'notification_bundle_base' => 1,
+ 'notification_read_timestamp' => null,
+ 'event_type' => $type,
+ 'notification_event = event_id'
+ ),
+ __METHOD__
+ );
+
+ $eventIds = array();
+ foreach ( $res as $row ) {
+ $eventIds[$row->notification_event] =
$row->notification_event;
+ }
+
+ return $eventIds;
+ }
}
diff --git a/includes/EchoBackend.php b/includes/EchoBackend.php
index e63e7fa..c5dc4b1 100644
--- a/includes/EchoBackend.php
+++ b/includes/EchoBackend.php
@@ -117,8 +117,16 @@
* Retrieves number of unread notifications that a user has.
* @param $user User object to check notifications for
* @param $dbSource string use master or slave storage to pull count
- * @return ResultWrapper|bool
+ * @return int
*/
abstract public function getNotificationCount( $user, $dbSource );
+ /**
+ * Get the event ids for corresponding unread notifications for an
+ * event type, return empty array if the notifcation count is greater
+ * than max count
+ * @param $user User object to check notification for
+ * @param $type string event type
+ */
+ abstract public function getUnreadNotifications( $user, $type );
}
diff --git a/includes/NotifUser.php b/includes/NotifUser.php
new file mode 100644
index 0000000..3187443
--- /dev/null
+++ b/includes/NotifUser.php
@@ -0,0 +1,230 @@
+<?php
+
+/**
+ * Entity that represents a notification target user
+ */
+class MWEchoNotifUser {
+
+ /**
+ * Notification target user
+ * @var User
+ */
+ private $mUser;
+
+ /**
+ * Cache object
+ * @var BagOStuff
+ */
+ private $cache;
+
+ /**
+ * Meta data for cache
+ * @var array
+ */
+ private $cacheMeta;
+
+ /**
+ * Constructor for initialization
+ */
+ private function __construct( User $user ) {
+ global $wgMemc, $wgEchoConfig;
+ $this->mUser = $user;
+ $this->cache = $wgMemc;
+
+ $this->cacheMeta = array(
+ 'talk-notification' => array(
+ 'key' => wfMemcKey( 'echo-edit-user-talk',
'event-id', $this->mUser->getId(), $wgEchoConfig['version'] ),
+ 'expiration' => 60 * 60 * 24 * 10,
+ ),
+ 'notification-count' => array(
+ 'key' => wfMemcKey( 'echo-notification-count',
$this->mUser->getId(), $wgEchoConfig['version'] ),
+ 'expiration' => 60 * 60 * 24
+ ),
+ );
+ }
+
+ /**
+ * Factory method
+ */
+ public static function newFromUser( User $user ) {
+ if ( $user->isAnon() ) {
+ throw new MWException( 'User must login to view
notification!' );
+ }
+ return new MWEchoNotifUser( $user );
+ }
+
+ /**
+ * Overwrite default setup
+ */
+ public function setValue( $key, $value ) {
+ $this->$key = $value;
+ }
+
+ /**
+ * Clear talk page notification when a user visits their talk page,
this would
+ * mark the notification as read if the total notification count is
less than 100,
+ * otherwise this would do nothing since talk page notification is
bundled and
+ * decrementing 1 would not affect 99+
+ * @param $type string the notification type
+ * @param $conds array conditions for clearing a notification
+ */
+ public function clearTalkNotification() {
+ global $wgEchoBackend;
+
+ $key = $this->cacheMeta['talk-notification']['key'];
+ $eventIds = $this->cache->get( $key );
+
+ // Nothing in cache, try to query the storage if needed
+ if ( $eventIds === false ) {
+ $eventIds = $this->getUnreadNotifications(
'edit-user-talk' );
+ }
+
+ if ( $eventIds ) {
+ $this->markRead( $eventIds );
+ $this->cache->set( $key, array(),
$this->cacheMeta['talk-notification']['expiration'] );
+ }
+ }
+
+ /**
+ * Cache talk page notification for this user
+ * @param $eventIds array|int
+ */
+ public function cacheTalkNotification( $eventIds ) {
+ if ( !is_array( $eventIds ) ) {
+ $eventIds = array( $eventIds => $eventIds );
+ }
+ $key = $this->cacheMeta['talk-notification']['key'];
+
+ $existing = $this->cache->get( $key );
+
+ // talk page notification is bundleld and hence there should be
only one
+ // unread notification, keep the cache with a max of 5 records
to handle
+ // possible race conditions
+ if ( is_array( $existing ) ) {
+ if ( count( $existing ) > 5 ) {
+ array_shift( $existing );
+ }
+ } else {
+ $existing = $this->getUnreadNotifications(
'edit-user-talk' );
+ }
+
+ if ( $existing ) {
+ $eventIds = $existing + $eventIds;
+ }
+
+ $this->cache->set( $key, $eventIds,
$this->cacheMeta['talk-notification']['expiration'] );
+ }
+
+ /**
+ * Get event ids for corresponding unread notifications for an event
type,
+ * return empty array if the notification count is greater than max
notification
+ * count
+ * @return array
+ */
+ protected function getUnreadNotifications( $type ) {
+ global $wgEchoBackend, $wgEchoMaxNotificationCount;
+
+ $count = $this->getNotificationCount();
+
+ $eventIds = array();
+
+ // if there are 99+ notifications, it is not necessary to
decrement the count
+ if ( $count <= $wgEchoMaxNotificationCount ) {
+ $eventIds = $wgEchoBackend->getUnreadNotifications(
$this->mUser, $type );
+ }
+
+ return $eventIds;
+ }
+
+ /**
+ * Retrieves number of unread notifications that a user has, would
return
+ * $wgEchoMaxNotificationCount + 1 at most
+ *
+ * @param $cached bool Set to false to bypass the cache.
+ * @param $dbSource string use master or slave database to pull count
+ * @return Integer: Number of unread notifications.
+ */
+ public function getNotificationCount( $cached = true, $dbSource =
DB_SLAVE ) {
+ global $wgEchoBackend, $wgEchoConfig;
+
+ if ( $this->mUser->isAnon() ) {
+ return 0;
+ }
+
+ $memcKey = $this->cacheMeta['notification-count']['key'];
+
+ if ( $cached && $this->cache->get( $memcKey ) !== false ) {
+ return (int)$this->cache->get( $memcKey );
+ }
+
+ $count = $wgEchoBackend->getNotificationCount( $this->mUser,
$dbSource );
+
+ $this->cache->set( $memcKey, $count,
$this->cacheMeta['notification-count']['expiration'] );
+
+ return (int)$count;
+ }
+
+ /**
+ * Mark one or more notifications read for a user.
+ *
+ * @param $eventIDs Array of event IDs to mark read
+ */
+ public function markRead( $eventIDs ) {
+ global $wgEchoBackend;
+
+ $eventIDs = array_filter( (array)$eventIDs, 'is_numeric' );
+ if ( !$eventIDs || wfReadOnly() ) {
+ return;
+ }
+ $wgEchoBackend->markRead( $this->mUser, $eventIDs );
+ $this->resetNotificationCount( DB_MASTER );
+ }
+
+ /**
+ * Attempt to mark all notifications as read
+ *
+ * @return boolean
+ */
+ public function markAllRead() {
+ global $wgEchoBackend, $wgEchoMaxNotificationCount;
+
+ if ( wfReadOnly() ) {
+ return false;
+ }
+
+ $notificationCount = $this->getNotificationCount();
+ // Only update all the unread notifications if it isn't a huge
number.
+ // TODO: Implement batched jobs it's over the maximum.
+ if ( $notificationCount <= $wgEchoMaxNotificationCount ) {
+ $wgEchoBackend->markAllRead( $this->mUser );
+ $this->resetNotificationCount( DB_MASTER );
+ return true;
+ } else {
+ return false;
+ }
+ }
+
+ /**
+ * Recalculates the number of notifications that a user has.
+ *
+ * @param $dbSource string use master or slave database to pull count
+ */
+ public function resetNotificationCount( $dbSource = DB_SLAVE ) {
+ $this->getNotificationCount( false, $dbSource );
+ $this->mUser->invalidateCache();
+ }
+
+ /**
+ * Retrieves formatted number of unread notifications that a user has.
+ *
+ * @param $cached bool Set to false to bypass the cache.
+ * @param $dbSource string use master or slave database to pull count
+ * @return String: Number of unread notifications.
+ */
+ public function getFormattedNotificationCount( $cached = true,
$dbSource = DB_SLAVE ) {
+ return EchoNotificationController::formatNotificationCount(
+ $this->getNotificationCount( $cached, $dbSource
)
+ );
+ }
+
+}
diff --git a/model/Notification.php b/model/Notification.php
index c32d6c0..d0a6abc 100644
--- a/model/Notification.php
+++ b/model/Notification.php
@@ -104,6 +104,8 @@
}
$wgEchoBackend->createNotification( $row );
+
+ wfRunHooks( 'EchoCreateNotificationComplete', array( $this ) );
}
/**
diff --git a/special/SpecialNotifications.php b/special/SpecialNotifications.php
index b7d6f66..e74c448 100644
--- a/special/SpecialNotifications.php
+++ b/special/SpecialNotifications.php
@@ -104,7 +104,7 @@
$out->addExtensionStyle(
"$wgExtensionAssetsPath/Echo/modules/base/ext.echo.base.css" );
// Mark items as read
if ( $unread ) {
- EchoNotificationController::markRead( $user, $unread );
+ MWEchoNotifUser::newFromUser( $user )->markRead(
$unread );
}
}
diff --git a/tests/NotifUserTest.php b/tests/NotifUserTest.php
new file mode 100644
index 0000000..498cbd7
--- /dev/null
+++ b/tests/NotifUserTest.php
@@ -0,0 +1,22 @@
+<?php
+
+class MWEchoNotifUserTest extends MediaWikiTestCase {
+
+ protected $notifUser;
+
+ protected function setUp() {
+ parent::setUp();
+ $this->setMwGlobals( 'wgMemc', new HashBagOStuff() );
+ }
+
+ public function testNewFromUser() {
+ $this->notifUser = MWEchoNotifUser::newFromUser(
User::newFromId( 2 ) );
+ $this->assertInstanceOf( 'MWEchoNotifUser', $this->notifUser );
+ }
+
+ /**
+ * @depends testNewFromUser
+ */
+ public function testClearTalkNotification() {
+
+ }
--
To view, visit https://gerrit.wikimedia.org/r/65349
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iadfe3cf7927d5318f89ba17f067000f9399060af
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Bsitu <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
