From 9f37ba05119767a0c28b03411632268e5a967625 Mon Sep 17 00:00:00 2001
From: Stephane Bisson <sbisson@wikimedia.org>
Date: Thu, 4 Oct 2018 17:26:03 -0400
Subject: [PATCH] Handle revision not found

Some Echo events include 'revid' in their extra info.
It can be used to check if a revision is minor in order
to respect the 'enotifminoredits' preference.

This patch ensures that it is not trying to call a function on
a null revision reference and it logs to debug to help
troubleshooting.

Also encapsulate the "is minor" check to a private function
to (hopefully) make the relationship with the preference
more clear.

Bug: T204795
Change-Id: I28a4c54f610dccc1356f6af0de9c2623d7bf94f0
---
 .../controller/NotificationController.php     | 46 +++++++++++++++----
 1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/includes/controller/NotificationController.php b/includes/controller/NotificationController.php
index a7540a271..5242e410d 100644
--- a/includes/controller/NotificationController.php
+++ b/includes/controller/NotificationController.php
@@ -1,4 +1,7 @@
 <?php
+use MediaWiki\Logger\LoggerFactory;
+use MediaWiki\MediaWikiServices;
+use MediaWiki\Storage\RevisionStore;
 
 /**
  * This class represents the controller for notifications
@@ -99,15 +102,11 @@ class EchoNotificationController {
 			$userNotifyTypes = $notifyTypes;
 			// Respect the enotifminoredits preference
 			// @todo should this be checked somewhere else?
-			if ( !$user->getOption( 'enotifminoredits' ) ) {
-				$extra = $event->getExtra();
-				if ( !empty( $extra['revid'] ) ) {
-					$rev = Revision::newFromID( $extra['revid'], Revision::READ_LATEST );
-
-					if ( $rev->isMinor() ) {
-						$notifyTypes = array_diff( $notifyTypes, [ 'email' ] );
-					}
-				}
+			if (
+				!$user->getOption( 'enotifminoredits' ) &&
+				self::hasMinorRevision( $event )
+			) {
+				$notifyTypes = array_diff( $notifyTypes, [ 'email' ] );
 			}
 			Hooks::run( 'EchoGetNotificationTypes', [ $user, $event, &$userNotifyTypes ] );
 
@@ -131,6 +130,35 @@ class EchoNotificationController {
 		}
 	}
 
+	/**
+	 * Check if an event is associated with a minor revision.
+	 *
+	 * @param EchoEvent $event
+	 * @return bool
+	 */
+	private static function hasMinorRevision( EchoEvent $event ) {
+		$revId = $event->getExtraParam( 'revid' );
+		if ( !$revId ) {
+			return false;
+		}
+
+		$revisionStore = MediaWikiServices::getInstance()->getRevisionStore();
+		$rev = $revisionStore->getRevisionById( $revId, RevisionStore::READ_LATEST );
+		if ( !$rev ) {
+			$logger = LoggerFactory::getInstance( 'Echo' );
+			$logger->debug(
+				'Notifying for event {eventId}. Revision \'{revId}\' not found.',
+				[
+					'eventId' => $event->getId(),
+					'revId' => $revId,
+				]
+			);
+			return false;
+		}
+
+		return $rev->isMinor();
+	}
+
 	/**
 	 * Schedule a job to check and delete older notifications
 	 *