Allow presentation models to indicate a notification can't be formatted

Adds EchoEventPresentationModel::canRender() for notification types to
indicate that something can't be rendered if for example, a page is
deleted.

In that case, the notification is marked as read in a deferred update.
All callers were also updated to check if the notification was formatted
properly.

Bug: T116888
Change-Id: Idb975feaec893ef86c41cc487102e3539c07e328

includes/DataOutputFormatter.php

@@ -19,7 +19,7 @@ class EchoDataOutputFormatter {
 	 * @param string|bool $format specifify output format, false to not format any notifications
 	 * @param User $user the target user viewing the notification
 	 * @param Language $lang Language to format the notification in
-	 * @return array
+	 * @return array|bool false if it could not be formatted
 	 */
 	public static function formatOutput( EchoNotification $notification, $format = false, User $user, Language $lang ) {
 		$event = $notification->getEvent();
@@ -109,12 +109,25 @@ class EchoDataOutputFormatter {
 		}
 
 		if ( $format ) {
-			$output['*'] = self::formatNotification( $event, $user, $format, $lang );
+			$formatted = self::formatNotification( $event, $user, $format, $lang );
+			if ( $formatted === false ) {
+				// Can't display it, so mark it as read
+				EchoDeferredMarkAsReadUpdate::add( $event, $user );
+				return false;
+			}
+			$output['*'] = $formatted;
 		}
 
 		return $output;
 	}
 
+	/**
+	 * @param EchoEvent $event
+	 * @param User $user
+	 * @param $format
+	 * @param Language $lang
+	 * @return string|bool false if it could not be formatted
+	 */
 	protected static function formatNotification( EchoEvent $event, User $user, $format, $lang ) {
 		if ( isset( self::$formatters[$format] )
 			&& EchoEventPresentationModel::supportsPresentationModel( $event->getType() )

includes/DeferredMarkAsReadUpdate.php

@@ -14,7 +14,20 @@ class EchoDeferredMarkAsReadUpdate implements DeferrableUpdate {
 	 * @param EchoEvent $event
 	 * @param User $user
 	 */
-	public function add( EchoEvent $event, User $user ) {
+	public static function add( EchoEvent $event, User $user ) {
+		static $update;
+		if ( $update === null ) {
+			$update = new self();
+			DeferredUpdates::addUpdate( $update );
+		}
+		$update->addInternal( $event, $user );
+	}
+
+	/**
+	 * @param EchoEvent $event
+	 * @param User $user
+	 */
+	private function addInternal( EchoEvent $event, User $user ) {
 		$uid = $user->getId();
 		if ( isset( $this->events[$uid] ) ) {
 			$this->events[$uid]['eventIds'][] = $event->getId();

includes/api/ApiEchoNotifications.php

@@ -145,9 +145,10 @@ class ApiEchoNotifications extends ApiQueryBase {
 
 		wfProfileIn( __METHOD__ . '-formatting' );
 		foreach ( $notifs as $notif ) {
-			$result['list'][$notif->getEvent()->getID()] = EchoDataOutputFormatter::formatOutput(
+			$output = EchoDataOutputFormatter::formatOutput( $notif, $format, $user, $this->getLanguage() );
-				$notif, $format, $user, $this->getLanguage()
+			if ( $output !== false ) {
-			);
+				$result['list'][$notif->getEvent()->getID()] = $output;
+			}
 		}
 		wfProfileOut( __METHOD__ . '-formatting' );
 

includes/controller/NotificationController.php

@@ -21,13 +21,6 @@ class EchoNotificationController {
 	 */
 	static protected $userWhitelist;
 
-	/**
-	 * Queue's that failed formatting and marks them as read at end of request.
-	 *
-	 * @var EchoDeferredMarkAsReadUpdate|null
-	 */
-	static protected $markAsRead;
-
 	/**
 	 * Format the notification count with Language::formatNum().  In addition, for large count,
 	 * return abbreviated version, e.g. 99+
@@ -410,12 +403,8 @@ class EchoNotificationController {
 		// FIXME: The only issue is that the badge count won't be up to date
 		// till you refresh the page.  Probably we could do this in the browser
 		// so that if the formatting is empty and the notif is unread, put it
-		// in the auto-mark-read API
+		// in the auto-mark-read APIs
-		if ( self::$markAsRead === null ) {
+		EchoDeferredMarkAsReadUpdate::add( $event, $user );
-			self::$markAsRead = new EchoDeferredMarkAsReadUpdate();
-			DeferredUpdates::addUpdate( self::$markAsRead );
-		}
-		self::$markAsRead->add( $event, $user );
 	}
 
 	/**

includes/formatters/EchoFlyoutFormatter.php

@@ -10,6 +10,9 @@
 class EchoFlyoutFormatter extends EchoEventFormatter {
 	public function format( EchoEvent $event ) {
 		$model = EchoEventPresentationModel::factory( $event, $this->language, $this->user );
+		if ( !$model->canRender() ) {
+			return false;
+		}
 
 		$icon = Html::element(
 			'img',

includes/formatters/EventPresentationModel.php

@@ -124,6 +124,18 @@ abstract class EchoEventPresentationModel {
 		}
 	}
 
+	/**
+	 * To be overridden by subclasses if they are unable to render the
+	 * notification, for example when a page is deleted.
+	 * If this function returns false, no other methods will be called
+	 * on the object.
+	 *
+	 * @return bool
+	 */
+	public function canRender() {
+		return true;
+	}
+
 	/**
 	 * @return string Message key that will be used in getHeaderMessage
 	 */

includes/formatters/MentionPresentationModel.php

@@ -27,6 +27,10 @@ class EchoMentionPresentationModel extends EchoEventPresentationModel {
 		return $this->sectionTitle;
 	}
 
+	public function canRender() {
+		return (bool)$this->event->getTitle();
+	}
+
 	/**
 	 * Override to switch the message key to -nosection
 	 * if no section title was detected

includes/special/SpecialNotifications.php

@@ -51,7 +51,10 @@ class SpecialNotifications extends SpecialPage {
 			$attributeManager->getUserEnabledEvents( $user, 'web' )
 		);
 		foreach ( $notifications as $notification ) {
-			$notif[] = EchoDataOutputFormatter::formatOutput( $notification, 'html', $user, $this->getLanguage() );
+			$output = EchoDataOutputFormatter::formatOutput( $notification, 'html', $user, $this->getLanguage() );
+			if ( $output ) {
+				$notif[] = $output;
+			}
 		}
 
 		// If there are no notifications, display a message saying so