Legoktm has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/249444

Change subject: Allow presentation models to indicate a notification can't be 
formatted
......................................................................

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
---
M includes/DataOutputFormatter.php
M includes/DeferredMarkAsReadUpdate.php
M includes/api/ApiEchoNotifications.php
M includes/controller/NotificationController.php
M includes/formatters/EchoFlyoutFormatter.php
M includes/formatters/EventPresentationModel.php
M includes/formatters/MentionPresentationModel.php
M includes/special/SpecialNotifications.php
8 files changed, 55 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Echo 
refs/changes/44/249444/1

diff --git a/includes/DataOutputFormatter.php b/includes/DataOutputFormatter.php
index 01d62d1..8eacc18 100644
--- a/includes/DataOutputFormatter.php
+++ b/includes/DataOutputFormatter.php
@@ -18,7 +18,7 @@
         * @param EchoNotification $notification
         * @param string|bool $format specifify output format, false to not 
format any notifications
         * @param User|null $user the target user viewing the notification
-        * @return array
+        * @return array|bool false if it could not be formatted
         */
        public static function formatOutput( EchoNotification $notification, 
$format = false, User $user = null ) {
                $event = $notification->getEvent();
@@ -113,12 +113,24 @@
                }
 
                if ( $format ) {
-                       $output['*'] = self::formatNotification( $event, $user, 
$format );
+                       $formatted = self::formatNotification( $event, $user, 
$format );
+                       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
+        * @return string|bool false if it could not be formatted
+        */
        protected static function formatNotification( EchoEvent $event, User 
$user, $format ) {
                global $wgLang;
                if ( isset( self::$formatters[$format] )
diff --git a/includes/DeferredMarkAsReadUpdate.php 
b/includes/DeferredMarkAsReadUpdate.php
index 9b02e8c..decde62 100644
--- a/includes/DeferredMarkAsReadUpdate.php
+++ b/includes/DeferredMarkAsReadUpdate.php
@@ -14,7 +14,20 @@
         * @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();
diff --git a/includes/api/ApiEchoNotifications.php 
b/includes/api/ApiEchoNotifications.php
index db2b03c..fa7c565 100644
--- a/includes/api/ApiEchoNotifications.php
+++ b/includes/api/ApiEchoNotifications.php
@@ -146,7 +146,10 @@
 
                wfProfileIn( __METHOD__ . '-formatting' );
                foreach ( $notifs as $notif ) {
-                       $result['list'][$notif->getEvent()->getID()] = 
EchoDataOutputFormatter::formatOutput( $notif, $format, $user );
+                       $output = EchoDataOutputFormatter::formatOutput( 
$notif, $format, $user );
+                       if ( $output !== false ) {
+                               $result['list'][$notif->getEvent()->getID()] = 
$output;
+                       }
                }
                wfProfileOut( __METHOD__ . '-formatting' );
 
diff --git a/includes/controller/NotificationController.php 
b/includes/controller/NotificationController.php
index 5f6a2e0..c62d8ca 100644
--- a/includes/controller/NotificationController.php
+++ b/includes/controller/NotificationController.php
@@ -22,13 +22,6 @@
        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 @@
                // 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
-               if ( self::$markAsRead === null ) {
-                       self::$markAsRead = new EchoDeferredMarkAsReadUpdate();
-                       DeferredUpdates::addUpdate( self::$markAsRead );
-               }
-               self::$markAsRead->add( $event, $user );
+               // in the auto-mark-read APIs
+               EchoDeferredMarkAsReadUpdate::add( $event, $user );
        }
 
        /**
diff --git a/includes/formatters/EchoFlyoutFormatter.php 
b/includes/formatters/EchoFlyoutFormatter.php
index dae1c98..ef02dd6 100644
--- a/includes/formatters/EchoFlyoutFormatter.php
+++ b/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',
diff --git a/includes/formatters/EventPresentationModel.php 
b/includes/formatters/EventPresentationModel.php
index b5bacac..b798c62 100644
--- a/includes/formatters/EventPresentationModel.php
+++ b/includes/formatters/EventPresentationModel.php
@@ -125,6 +125,16 @@
        }
 
        /**
+        * To be overridden by subclasses if they are unable to render the
+        * notification, for example when a page is deleted
+        *
+        * @return bool
+        */
+       public function canRender() {
+               return true;
+       }
+
+       /**
         * @return string Message key that will be used in getHeaderMessage
         */
        protected function getHeaderMessageKey() {
diff --git a/includes/formatters/MentionPresentationModel.php 
b/includes/formatters/MentionPresentationModel.php
index 5cdfc3a..d07f329 100644
--- a/includes/formatters/MentionPresentationModel.php
+++ b/includes/formatters/MentionPresentationModel.php
@@ -27,6 +27,10 @@
                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
diff --git a/includes/special/SpecialNotifications.php 
b/includes/special/SpecialNotifications.php
index 60e344f..ff83a63 100644
--- a/includes/special/SpecialNotifications.php
+++ b/includes/special/SpecialNotifications.php
@@ -51,7 +51,10 @@
                        $attributeManager->getUserEnabledEvents( $user, 'web' )
                );
                foreach ( $notifications as $notification ) {
-                       $notif[] = EchoDataOutputFormatter::formatOutput( 
$notification, 'html', $user );
+                       $output = EchoDataOutputFormatter::formatOutput( 
$notification, 'html', $user );
+                       if ( $output ) {
+                               $notif[] = $output;
+                       }
                }
 
                // If there are no notifications, display a message saying so

-- 
To view, visit https://gerrit.wikimedia.org/r/249444
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idb975feaec893ef86c41cc487102e3539c07e328
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Legoktm <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to