Replace mark-as-read-on-click with ?markasread= URL parameter

Notifications were being marked as read in response to a click event
in JavaScript, but that causes a jarring effect in the UI, and it's
not reliable (the browser could abort the AJAX request).

Instead, add ?markasread=XYZ to the end of every primary link URL,
where XYZ is the event ID.

Bug: T133975
Depends-On: Icb99d5479836fea25a47451b5a758dd71f642f71
Change-Id: I8047d121584b43e6172463a50ad0e0de5f7fa73c

Hooks.php

@@ -762,11 +762,11 @@ class EchoHooks {
 		// @todo should this really be here?
 		$subtractAlerts = 0;
 		$subtractMessages = 0;
+		$eventIds = array();
 		if ( $title->getArticleID() ) {
-			$mapper = new EchoTargetPageMapper();
-			$fetchedTargetPages = $mapper->fetchByUserPageId( $user, $title->getArticleID() );
+			$targetPageMapper = new EchoTargetPageMapper();
+			$fetchedTargetPages = $targetPageMapper->fetchByUserPageId( $user, $title->getArticleID() );
 			if ( $fetchedTargetPages ) {
-				$eventIds = array();
 				$attribManager = EchoAttributeManager::newFromGlobalVars();
 				/* @var EchoTargetPage[] $targetPages */
 				foreach ( $fetchedTargetPages as $id => $targetPages ) {
@@ -776,19 +776,35 @@ class EchoHooks {
 						$targetPages[0]->getEventType()
 					);
 					if ( $section === EchoAttributeManager::MESSAGE ) {
-						$subtractMessages += 1;
+						$subtractMessages++;
 					} else {
 						// ALERT
-						$subtractAlerts += 1;
+						$subtractAlerts++;
 					}
 					$eventIds[] = $id;
 				}
-				DeferredUpdates::addCallableUpdate( function () use ( $user, $eventIds ) {
-					$notifUser = MWEchoNotifUser::newFromUser( $user );
-					$notifUser->markRead( $eventIds );
-				} );
 			}
 		}
+		// Attempt to mark as read the event ID in the ?markasread= parameter, if present
+		$markAsReadId = $sk->getOutput()->getRequest()->getInt( 'markasread' );
+		if ( $markAsReadId !== 0 && !in_array( $markAsReadId, $eventIds ) ) {
+			$notifMapper = new EchoNotificationMapper();
+			$notif = $notifMapper->fetchByUserEvent( $user, $markAsReadId );
+			if ( $notif && !$notif->getReadTimestamp() ) {
+				if ( $notif->getEvent()->getSection() === EchoAttributeManager::MESSAGE ) {
+					$subtractMessages++;
+				} else {
+					$subtractAlerts++;
+				}
+				$eventIds[] = $markAsReadId;
+			}
+		}
+		if ( $eventIds ) {
+			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 );

Resources.php

@@ -205,7 +205,10 @@ $wgResourceModules += array(
 		'scripts' => array(
 			'ext.echo.init.js',
 		),
-		'dependencies' => array( 'ext.echo.api' ),
+		'dependencies' => array(
+			'ext.echo.api',
+			'mediawiki.Uri',
+		 ),
 		'targets' => array( 'desktop' ),
 	),
 	// Base no-js styles

includes/formatters/EchoFlyoutFormatter.php

@@ -50,7 +50,7 @@ class EchoFlyoutFormatter extends EchoEventFormatter {
 		) . "\n";
 
 		// Add the primary link afterwards, if it has one
-		$primaryLink = $model->getPrimaryLink();
+		$primaryLink = $model->getPrimaryLinkWithMarkAsRead();
 		if ( $primaryLink !== false ) {
 			$html .= Html::element(
 				'a',

includes/formatters/EventPresentationModel.php

@@ -326,6 +326,19 @@ abstract class EchoEventPresentationModel implements JsonSerializable {
 	 */
 	abstract public function getPrimaryLink();
 
+	/**
+	 * Like getPrimaryLink(), but with the URL altered to add ?markasread=XYZ. When this link is followed,
+	 * the notification is marked as read.
+	 * @return array|bool
+	 */
+	public function getPrimaryLinkWithMarkAsRead() {
+		$primaryLink = $this->getPrimaryLink();
+		if ( $primaryLink ) {
+			$primaryLink['url'] = wfAppendQuery( $primaryLink['url'], array( 'markasread' => $this->event->getId() ) );
+		}
+		return $primaryLink;
+	}
+
 	/**
 	 * Array of secondary link details, including possibly-relative URLs, label,
 	 * description & icon name.
@@ -358,7 +371,7 @@ abstract class EchoEventPresentationModel implements JsonSerializable {
 			'body' => $body ? $body->escaped() : '',
 			'icon' => $this->getIconType(),
 			'links' => array(
-				'primary' => $this->getPrimaryLink() ?: array(),
+				'primary' => $this->getPrimaryLinkWithMarkAsRead() ?: array(),
 				'secondary' => array_values( array_filter( $this->getSecondaryLinks() ) ),
 			),
 		);

includes/formatters/SpecialNotificationsFormatter.php

@@ -38,7 +38,7 @@ class SpecialNotificationsFormatter extends EchoEventFormatter {
 
 		// Add links to the footer, primary goes first, then secondary ones
 		$links = array();
-		$primaryLink = $model->getPrimaryLink();
+		$primaryLink = $model->getPrimaryLinkWithMarkAsRead();
 		if ( $primaryLink !== false ) {
 			$links[] = $primaryLink;
 		}

includes/mapper/NotificationMapper.php

@@ -260,6 +260,34 @@ class EchoNotificationMapper extends EchoAbstractMapper {
 		}
 	}
 
+	/**
+	 * Create an EchoNotification by user and event ID.
+	 *
+	 * @param User $user
+	 * @param int $eventID
+	 * @return EchoNotification|bool
+	 */
+	public function fetchByUserEvent( User $user, $eventId ) {
+		$dbr = $this->dbFactory->getEchoDb( DB_SLAVE );
+
+		$row = $dbr->selectRow(
+			array( 'echo_notification', 'echo_event' ),
+			'*',
+			array(
+				'notification_user' => $user->getId(),
+				'notification_event' => $eventId
+			),
+			 __METHOD__
+		 );
+
+		if ( $row ) {
+			return EchoNotification::newFromRow( $row );
+		} else {
+			return false;
+		}
+	}
+
+
 	/**
 	 * Fetch a notification by user in the specified offset.  The caller should
 	 * know that passing a big number for offset is NOT going to work

modules/ext.echo.init.js

@@ -1,6 +1,13 @@
 ( function ( mw, $ ) {
 	'use strict';
 
+	// Remove ?markasread=XYZ from the URL
+	var uri = new mw.Uri();
+	if ( uri.query.markasread !== undefined ) {
+		delete uri.query.markasread;
+		window.history.replaceState( null, document.title, uri );
+	}
+
 	mw.echo = mw.echo || {};
 
 	// Activate ooui

modules/ooui/mw.echo.ui.NotificationItemWidget.js

@@ -198,9 +198,6 @@
 								// Source of this notification if it is cross-wiki
 								widget.bundle ? widget.getModel().getSource() : ''
 							);
-
-							// Toggle the notification as read
-							widget.model.toggleRead( true );
 						} )
 				);
 		}