jenkins-bot has submitted this change and it was merged.
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, 58 insertions(+), 20 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/DataOutputFormatter.php b/includes/DataOutputFormatter.php
index 1230c1a..ba54584 100644
--- a/includes/DataOutputFormatter.php
+++ b/includes/DataOutputFormatter.php
@@ -19,7 +19,7 @@
* @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 @@
}
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() )
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 d0efeed..60c29ba 100644
--- a/includes/api/ApiEchoNotifications.php
+++ b/includes/api/ApiEchoNotifications.php
@@ -145,9 +145,10 @@
wfProfileIn( __METHOD__ . '-formatting' );
foreach ( $notifs as $notif ) {
- $result['list'][$notif->getEvent()->getID()] =
EchoDataOutputFormatter::formatOutput(
- $notif, $format, $user, $this->getLanguage()
- );
+ $output = EchoDataOutputFormatter::formatOutput(
$notif, $format, $user, $this->getLanguage() );
+ 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..8cdc818 100644
--- a/includes/formatters/EventPresentationModel.php
+++ b/includes/formatters/EventPresentationModel.php
@@ -125,6 +125,18 @@
}
/**
+ * 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
*/
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 51c18fe..0dcbcb7 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, $this->getLanguage() );
+ $output = EchoDataOutputFormatter::formatOutput(
$notification, 'html', $user, $this->getLanguage() );
+ 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: merged
Gerrit-Change-Id: Idb975feaec893ef86c41cc487102e3539c07e328
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Legoktm <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: Mattflaschen <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
