Defer onPersonalUrls() DB writes to post-send (try #2)

This reverts commit e372f3ce6f.

The previous attempt was broken because
EchoTargetPageMapper::fetchByUserPageId() returns a list of
EchoTargetPage objects, not a single one.

Bug: T117531
Change-Id: Id02a025e3736a7b92d9d6fb8adf29ef674f8e2fa

Hooks.php

@@ -735,24 +735,42 @@ class EchoHooks {
 			return true;
 		}
 
-		// Attempt to mark a notification as read when visiting a page,
+		// Attempt to mark a notification as read when visiting a page
-		// ideally this should be deferred to end of request and update
+		// @todo should this really be here?
-		// the notification count accordingly
+		$subtractAlerts = 0;
-		// @Fixme - Find a better place to put this code
+		$subtractMessages = 0;
 		if ( $title->getArticleID() ) {
 			$mapper = new EchoTargetPageMapper();
-			$targetPages = $mapper->fetchByUserPageId( $user, $title->getArticleID() );
+			$fetchedTargetPages = $mapper->fetchByUserPageId( $user, $title->getArticleID() );
-			if ( $targetPages ) {
+			if ( $fetchedTargetPages ) {
-				$eventIds = array_keys( $targetPages );
+				$eventIds = array();
-				$notifUser = MWEchoNotifUser::newFromUser( $user );
+				$attribManager = EchoAttributeManager::newFromGlobalVars();
-				$notifUser->markRead( $eventIds );
+				/* @var EchoTargetPage[] $targetPages */
+				foreach ( $fetchedTargetPages as $id => $targetPages ) {
+					// Only look at the first target page since they'll
+					// all point to the same event
+					$section = $attribManager->getNotificationSection(
+						$targetPages[0]->getEventType()
+					);
+					if ( $section === EchoAttributeManager::MESSAGE ) {
+						$subtractMessages += 1;
+					} else {
+						// ALERT
+						$subtractAlerts += 1;
+					}
+					$eventIds[] = $id;
+				}
+				DeferredUpdates::addCallableUpdate( function () use ( $user, $eventIds ) {
+					$notifUser = MWEchoNotifUser::newFromUser( $user );
+					$notifUser->markRead( $eventIds );
+				} );
 			}
 		}
 
 		// Add a "My notifications" item to personal URLs
 		$notifUser = MWEchoNotifUser::newFromUser( $user );
-		$msgCount = $notifUser->getMessageCount();
+		$msgCount = $notifUser->getMessageCount() - $subtractMessages;
-		$alertCount = $notifUser->getAlertCount();
+		$alertCount = $notifUser->getAlertCount() - $subtractAlerts;
 
 		$msgNotificationTimestamp = $notifUser->getLastUnreadMessageTime();
 		$alertNotificationTimestamp = $notifUser->getLastUnreadAlertTime();

includes/mapper/TargetPageMapper.php

@@ -28,13 +28,15 @@ class EchoTargetPageMapper extends EchoAbstractMapper {
 		$dbr = $this->dbFactory->getEchoDb( DB_SLAVE );
 
 		$res = $dbr->select(
-			array( 'echo_target_page' ),
+			array( 'echo_target_page', 'echo_event' ),
-			self::$fields,
+			array_merge( self::$fields, array( 'event_type' ) ),
 			array(
 				'etp_user' => $user->getId(),
 				'etp_page' => $pageId
 			),
-			__METHOD__
+			__METHOD__,
+			array(),
+			array( 'echo_event' => array( 'JOIN', 'etp_event=event_id' ) )
 		);
 		if ( $res ) {
 			$targetPages = array();

includes/model/TargetPage.php

@@ -32,6 +32,11 @@ class EchoTargetPage extends EchoAbstractEntity {
 	 */
 	protected $eventId;
 
+	/**
+	 * @var string
+	 */
+	protected $eventType;
+
 	/**
 	 * Only allow creating instance internally
 	 */
@@ -44,7 +49,7 @@ class EchoTargetPage extends EchoAbstractEntity {
 	 * @param User $user
 	 * @param Title $title
 	 * @param EchoEvent $event
-	 * @return TargetPage|null
+	 * @return EchoTargetPage|null
 	 */
 	public static function create( User $user, Title $title, EchoEvent $event ) {
 		// This only support title with a page_id
@@ -55,6 +60,7 @@ class EchoTargetPage extends EchoAbstractEntity {
 		$obj->user = $user;
 		$obj->event = $event;
 		$obj->eventId = $event->getId();
+		$obj->eventType = $event->getType();
 		$obj->title = $title;
 		$obj->pageId = $title->getArticleID();
 
@@ -83,6 +89,9 @@ class EchoTargetPage extends EchoAbstractEntity {
 		$obj->user = User::newFromId( $row->etp_user );
 		$obj->pageId = $row->etp_page;
 		$obj->eventId = $row->etp_event;
+		if ( isset( $row->event_type ) ) {
+			$obj->eventType = $row->event_type;
+		}
 
 		return $obj;
 	}
@@ -130,6 +139,17 @@ class EchoTargetPage extends EchoAbstractEntity {
 		return $this->eventId;
 	}
 
+	/**
+	 * @return string
+	 */
+	public function getEventType() {
+		if ( !$this->eventType ) {
+			$this->eventType = $this->getEvent()->getType();
+		}
+
+		return $this->eventType;
+	}
+
 	/**
 	 * Convert the properties to a database row
 	 * @return array