(bug 47912) Visiting talk page should mark talk notif as read

This needs some more manual testing and adding unit testing

Change-Id: Iadfe3cf7927d5318f89ba17f067000f9399060af

Echo.php

@@ -85,6 +85,7 @@ $wgSpecialPageGroups['Notifications'] = 'users';
 $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['ResourceLoaderRegisterModules'][] = 'EchoHooks::onResourceLoaderRegist
 $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['EchoGetDefaultNotifiedUsers'][] = 'EchoHooks::getDefaultNotifiedUsers'
 $wgHooks['EchoGetNotificationTypes'][] = 'EchoHooks::getNotificationTypes';
 $wgHooks['EchoGetBundleRules'][] = 'EchoHooks::onEchoGetBundleRules';
 $wgHooks['EchoAbortEmailNotification'][] = 'EchoHooks::onEchoAbortEmailNotification';
+$wgHooks['EchoCreateNotificationComplete'][] = 'EchoHooks::onEchoCreateNotificationComplete';
 
 // Hook appropriate events
 $wgHooks['ArticleSaveComplete'][] = 'EchoHooks::onArticleSaved';

Hooks.php

@@ -626,7 +626,7 @@ class EchoHooks {
 			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 @@ class EchoHooks {
 		$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 @@ class EchoHooks {
 		// 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,35 @@ class EchoHooks {
 
 		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
+	 * @return bool true in all cases
+	 */
+	public static function onUserClearNewTalkNotification( User $user ) {
+		if ( !$user->isAnon() ) {
+			MWEchoNotifUser::newFromUser( $user )->clearTalkNotification();
+		}
+		return true;
+	}
+
+	/**
+	 * Handler for EchoCreateNotificationComplete hook, this will allow some
+	 * extra stuff to be done upon creating a new notification
+	 * @param $notif EchoNotification
+	 * @return bool true in all cases
+	 */
+	public static function onEchoCreateNotificationComplete( EchoNotification $notif ) {
+		if ( $notif->getEvent() && $notif->getUser() ) {
+			// Extra stuff for talk page notification
+			if ( $notif->getEvent()->getType() === 'edit-user-talk' ) {
+				$notifUser = MWEchoNotifUser::newFromUser( $notif->getUser() );
+				$notifUser->flagCacheWithNewTalkNotification();
+			}
+		}
+
+		return true;
+	}
 }

api/ApiEchoNotifications.php

@@ -11,15 +11,17 @@ class ApiEchoNotifications extends ApiQueryBase {
 			$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 @@ class ApiEchoNotifications extends ApiQueryBase {
 		}
 
 		if ( in_array( 'count', $prop ) ) {
-			$result['count'] = EchoNotificationController::getFormattedNotificationCount( $user );
+			$result['count'] = $notifUser->getFormattedNotificationCount();
 		}
 
 		if ( in_array( 'index', $prop ) ) {

controller/NotificationController.php

@@ -4,35 +4,6 @@ class EchoNotificationController {
 	static protected $blacklist;
 	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
@@ -135,20 +106,6 @@ class EchoNotificationController {
 		return 'other';
 	}
 
-	/**
-	 * 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+
@@ -170,57 +127,6 @@ class EchoNotificationController {
 		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();
-	}
-
 	/**
 	 * Processes notifications for a newly-created EchoEvent
 	 *

includes/DbEchoBackend.php

@@ -33,7 +33,7 @@ class MWDbEchoBackend extends MWEchoBackend {
 				$row['notification_timestamp'] = $dbw->timestamp( $row['notification_timestamp'] );
 				$dbw->insert( 'echo_notification', $row, $fname );
 				$dbw->commit( $fname );
-				EchoNotificationController::resetNotificationCount( $user, DB_MASTER );
+				MWEchoNotifUser::newFromUser( $user )->resetNotificationCount( DB_MASTER );
 			}
 		);
 	}
@@ -301,4 +301,34 @@ class MWDbEchoBackend extends MWEchoBackend {
 		return $db->numRows( $res );
 	}
 
+	/**
+	 * IMPORTANT: should only call this function if the number of unread notification
+	 * is reasonable, for example, unread notification count is less than the max
+	 * display defined in $wgEchoMaxNotificationCount
+	 * @param $user User
+	 * @param $type string
+	 * @return array
+	 */
+	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;
+	}
 }

includes/EchoBackend.php

@@ -118,8 +118,16 @@ abstract class MWEchoBackend {
 	 * 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
+	 * @param $user User object to check notification for
+	 * @param $type string event type
+	 * @return array
+	 */
+	abstract public function getUnreadNotifications( $user, $type );
 }

includes/NotifUser.php

@@ -0,0 +1,196 @@
+<?php
+
+/**
+ * Entity that represents a notification target user
+ */
+class MWEchoNotifUser {
+
+	/**
+	 * Notification target user
+	 * @var User
+	 */
+	private $mUser;
+
+	/**
+	 * Object cache
+	 * @var BagOStuff
+	 */
+	private $cache;
+
+	/**
+	 * Echo backend storage
+	 * @var MWEchoBackend
+	 */
+	private $storage;
+
+	/**
+	 * Constructor for initialization
+	 * @param $user User
+	 */
+	private function __construct( User $user ) {
+		global $wgMemc, $wgEchoBackend;
+		$this->mUser = $user;
+		$this->storage = $wgEchoBackend;
+		$this->cache = $wgMemc;
+	}
+
+	/**
+	 * Factory method
+	 * @param $user User
+	 * @throws MWException
+	 * @return MWEchoNotifUser
+	 */
+	public static function newFromUser( User $user ) {
+		if ( $user->isAnon() ) {
+			throw new MWException( 'User must be logged in to view notification!' );
+		}
+		return new MWEchoNotifUser( $user );
+	}
+
+	/**
+	 * Clear talk page notification when users visit their talk pages.  This
+	 * only resets if the notification count is less than max notification
+	 * count. If the user has 99+ notifications, decrementing 1 bundled talk
+	 * page notification would not really affect the count
+	 */
+	public function clearTalkNotification() {
+		// There is no new talk notification
+		if ( $this->cache->get( $this->getTalkNotificationCacheKey() ) === '0' ) {
+			return;
+		}
+
+		// Do nothing if the count display meets the max 99+
+		if ( $this->notifCountHasReachedMax() ) {
+			return;
+		}
+
+		// Mark the talk page notification as read
+		$this->markRead(
+			$this->storage->getUnreadNotifications(
+				$this->mUser,
+				'edit-user-talk'
+			)
+		);
+
+		$this->flagCacheWithNoTalkNotification();
+	}
+
+	/**
+	 * Flag the cache with new talk notification
+	 */
+	public function flagCacheWithNewTalkNotification() {
+		$this->cache->set( $this->getTalkNotificationCacheKey(), '1', 86400 );
+	}
+
+	/**
+	 * Flag the cache with no talk notification
+	 */
+	public function flagCacheWithNoTalkNotification() {
+		$this->cache->set( $this->getTalkNotificationCacheKey(), '0', 86400 );
+	}
+
+	/**
+	 * Memcache key for talk notification
+	 */
+	public function getTalkNotificationCacheKey() {
+		global $wgEchoConfig;
+
+		return wfMemcKey( 'echo-new-talk-notification', $this->mUser->getId(), $wgEchoConfig['version'] );
+	}
+
+	/**
+	 * Check if the user has more notification count than max count display
+	 * @return bool
+	 */
+	public function notifCountHasReachedMax() {
+		global $wgEchoMaxNotificationCount;
+
+		if ( $this->getNotificationCount() > $wgEchoMaxNotificationCount ) {
+			return true;
+		} else {
+			return false;
+		}
+	}
+
+	/**
+	 * 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 $wgEchoConfig;
+
+		if ( $this->mUser->isAnon() ) {
+			return 0;
+		}
+
+		$memcKey = wfMemcKey( 'echo-notification-count', $this->mUser->getId(), $wgEchoConfig['version'] );
+
+		if ( $cached && $this->cache->get( $memcKey ) !== false ) {
+			return (int)$this->cache->get( $memcKey );
+		}
+
+		$count = $this->storage->getNotificationCount( $this->mUser, $dbSource );
+
+		$this->cache->set( $memcKey, $count, 86400 );
+
+		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 ) {
+		$eventIds = array_filter( (array)$eventIds, 'is_numeric' );
+		if ( !$eventIds || wfReadOnly() ) {
+			return;
+		}
+
+		$this->storage->markRead( $this->mUser, $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->storage->markAllRead( $this->mUser );
+		$this->resetNotificationCount( DB_MASTER );
+		$this->flagCacheWithNoTalkNotification();
+		return true;
+
+	}
+
+	/**
+	 * 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 )
+		);
+	}
+
+}

model/Notification.php

@@ -104,6 +104,8 @@ class EchoNotification {
 		}
 
 		$wgEchoBackend->createNotification( $this->user, $row );
+
+		wfRunHooks( 'EchoCreateNotificationComplete', array( $this ) );
 	}
 
 	/**

special/SpecialNotifications.php

@@ -104,7 +104,7 @@ class SpecialNotifications extends SpecialPage {
 		$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 );
 		}
 	}
 

tests/NotifUserTest.php

@@ -0,0 +1,72 @@
+<?php
+
+class MWEchoNotifUserTest extends MediaWikiTestCase {
+
+	protected function setUp() {
+		parent::setUp();
+		$this->setMwGlobals( 'wgMemc', new HashBagOStuff() );
+	}
+
+	public function testNewFromUser() {
+		$exception = false;
+		try {
+			MWEchoNotifUser::newFromUser( User::newFromId( 0 ) );
+		} catch ( Exception $e ) {
+			$exception = true;
+			$this->assertEquals( "User must be logged in to view notification!",
+				$e->getMessage() );
+		}
+		$this->assertTrue( $exception, "Got exception" );
+
+		$notifUser = MWEchoNotifUser::newFromUser( User::newFromId( 2 ) );
+		$this->assertInstanceOf( 'MWEchoNotifUser', $notifUser );
+	}
+
+	public function testFlagCacheWithNewTalkNotification() {
+		global $wgMemc;
+
+		$notifUser = MWEchoNotifUser::newFromUser( User::newFromId( 2 ) );
+
+		$notifUser->flagCacheWithNewTalkNotification();
+		$this->assertEquals( '1', $wgMemc->get( $notifUser->getTalkNotificationCacheKey() ) );
+	}
+
+	public function testFlagCacheWithNoTalkNotification() {
+		global $wgMemc;
+
+		$notifUser = MWEchoNotifUser::newFromUser( User::newFromId( 2 ) );
+
+		$notifUser->flagCacheWithNoTalkNotification();
+		$this->assertEquals( '0', $wgMemc->get( $notifUser->getTalkNotificationCacheKey() ) );
+	}
+
+	public function testNotifCountHasReachedMax() {
+		global $wgEchoMaxNotificationCount;
+
+		$notifUser = MWEchoNotifUser::newFromUser( User::newFromId( 2 ) );
+
+		if ( $notifUser->getNotificationCount() > $wgEchoMaxNotificationCount ) {
+			$this->assertTrue( $notifUser->notifCountHasReachedMax() );
+		} else {
+			$this->assertFalse( $notifUser->notifCountHasReachedMax() );
+		}
+	}
+
+	public function testClearTalkNotification() {
+		global $wgMemc;
+
+		$notifUser = MWEchoNotifUser::newFromUser( User::newFromId( 2 ) );
+
+		$notifUser->flagCacheWithNewTalkNotification();
+
+		$hasMax = $notifUser->notifCountHasReachedMax();
+
+		$notifUser->clearTalkNotification();
+		if ( $hasMax ) {
+			$this->assertEquals( '1', $wgMemc->get( $notifUser->getTalkNotificationCacheKey() ) );
+		} else {
+			$this->assertEquals( '0', $wgMemc->get( $notifUser->getTalkNotificationCacheKey() ) );
+		}
+	}
+
+}