jenkins-bot has submitted this change and it was merged.
Change subject: Moderate notifications
......................................................................
Moderate notifications
* When a page is deleted, moderate associated
notifications. When it is undeleted, unmoderate.
* When a notification cannot be rendered
(canRender() returns false), moderate it.
The biggest advantage of moderation over mark-read-and-ignore
is that those notifications are filtered out at the database
level from this point on. They are not re-processed every single
time and do not affect the number of notifications returned by
the API.
Bug: T140327
Bug: T140836
Bug: T141463
Change-Id: Idefe78408fd584c13aaa9174cee3055539d92848
---
M Echo.php
M Hooks.php
M autoload.php
M includes/DataOutputFormatter.php
A includes/DeferredMarkAsDeletedUpdate.php
D includes/DeferredMarkAsReadUpdate.php
M includes/controller/NotificationController.php
M includes/mapper/EventMapper.php
M includes/mapper/NotificationMapper.php
M includes/model/Event.php
10 files changed, 151 insertions(+), 74 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/Echo.php b/Echo.php
index c66cabb..aa13cf2 100644
--- a/Echo.php
+++ b/Echo.php
@@ -83,6 +83,9 @@
$wgHooks['EmailUserComplete'][] = 'EchoHooks::onEmailUserComplete';
$wgHooks['LoginFormValidErrorMessages'][] =
'EchoHooks::onLoginFormValidErrorMessages';
$wgHooks['OutputPageCheckLastModified'][] =
'EchoHooks::onOutputPageCheckLastModified';
+$wgHooks['ArticleDeleteComplete'][] = 'EchoHooks::onArticleDeleteComplete';
+$wgHooks['ArticleUndelete'][] = 'EchoHooks::onArticleUndelete';
+
// Extension:UserMerge support
$wgHooks['UserMergeAccountFields'][] = 'EchoHooks::onUserMergeAccountFields';
diff --git a/Hooks.php b/Hooks.php
index f451c44..efd5fe4 100644
--- a/Hooks.php
+++ b/Hooks.php
@@ -721,12 +721,14 @@
continue;
}
+ $linkFromPageId =
$linksUpdate->mTitle->getArticleId();
EchoEvent::create( array(
'type' => 'page-linked',
'title' => $title,
'agent' => $user,
'extra' => array(
- 'link-from-page-id' =>
$linksUpdate->mTitle->getArticleId(),
+ 'target-page' =>
$linkFromPageId,
+ 'link-from-page-id' =>
$linkFromPageId,
'revid' => $revid,
)
) );
@@ -1304,4 +1306,24 @@
return true;
}
+ public static function onArticleDeleteComplete( WikiPage &$article,
User &$user, $reason, $articleId, Content $content = null, LogEntry $logEntry )
{
+ \DeferredUpdates::addCallableUpdate( function () use (
$articleId ) {
+ $eventMapper = new EchoEventMapper();
+ $eventIds = $eventMapper->fetchIdsByPage( $articleId );
+ EchoModerationController::moderate( $eventIds, true );
+ } );
+ return true;
+ }
+
+ public static function onArticleUndelete( Title $title, $create,
$comment, $oldPageId ) {
+ if ( $create ) {
+ \DeferredUpdates::addCallableUpdate( function () use (
$oldPageId ) {
+ $eventMapper = new EchoEventMapper();
+ $eventIds = $eventMapper->fetchIdsByPage(
$oldPageId );
+ EchoModerationController::moderate( $eventIds,
false );
+ } );
+ }
+ return true;
+ }
+
}
diff --git a/autoload.php b/autoload.php
index 65589ed..a828fc5 100644
--- a/autoload.php
+++ b/autoload.php
@@ -30,7 +30,7 @@
'EchoContainmentList' => __DIR__ . '/includes/ContainmentSet.php',
'EchoContainmentSet' => __DIR__ . '/includes/ContainmentSet.php',
'EchoDataOutputFormatter' => __DIR__ .
'/includes/DataOutputFormatter.php',
- 'EchoDeferredMarkAsReadUpdate' => __DIR__ .
'/includes/DeferredMarkAsReadUpdate.php',
+ 'EchoDeferredMarkAsDeletedUpdate' => __DIR__ .
'/includes/DeferredMarkAsDeletedUpdate.php',
'EchoDiffGroup' => __DIR__ . '/includes/DiffParser.php',
'EchoDiffParser' => __DIR__ . '/includes/DiffParser.php',
'EchoDiffParserTest' => __DIR__ . '/tests/phpunit/DiffParserTest.php',
diff --git a/includes/DataOutputFormatter.php b/includes/DataOutputFormatter.php
index 61c7c2f..d7b866e 100644
--- a/includes/DataOutputFormatter.php
+++ b/includes/DataOutputFormatter.php
@@ -136,7 +136,7 @@
$formatted = self::formatNotification( $event, $user,
$format, $lang );
if ( $formatted === false ) {
// Can't display it, so mark it as read
- EchoDeferredMarkAsReadUpdate::add( $event,
$user );
+ EchoDeferredMarkAsDeletedUpdate::add( $event );
return false;
}
$output['*'] = $formatted;
diff --git a/includes/DeferredMarkAsDeletedUpdate.php
b/includes/DeferredMarkAsDeletedUpdate.php
new file mode 100644
index 0000000..0531e89
--- /dev/null
+++ b/includes/DeferredMarkAsDeletedUpdate.php
@@ -0,0 +1,72 @@
+<?php
+use MediaWiki\Logger\LoggerFactory;
+
+/**
+ * Mark event notifications as deleted at the end of a request. Used to queue
up
+ * individual events to mark due to formatting failures.
+ */
+class EchoDeferredMarkAsDeletedUpdate implements DeferrableUpdate {
+ /**
+ * @var array
+ */
+ protected $events = array();
+
+ /**
+ * @param EchoEvent $event
+ */
+ public static function add( EchoEvent $event ) {
+ static $update;
+ if ( $update === null ) {
+ $update = new self();
+ DeferredUpdates::addUpdate( $update );
+ }
+ $update->addInternal( $event );
+ }
+
+ /**
+ * @param EchoEvent $event
+ */
+ private function addInternal( EchoEvent $event ) {
+ $this->events[] = $event;
+ }
+
+ private function filterEventsWithTitleDbLag() {
+ return array_filter(
+ $this->events,
+ function ( EchoEvent $event ) {
+ if ( !$event->getTitle() && $event->getTitle(
true ) ) {
+ // It is very likely this event was
found
+ // unreaderable because of slave lag.
+ // Do not moderate it at this time.
+ LoggerFactory::getInstance( 'Echo'
)->debug(
+
'EchoDeferredMarkAsDeletedUpdate: Event {eventId} was found unrenderable but
its associated title exists on Master. Skipping.',
+ array(
+ 'eventId' =>
$event->getId(),
+ 'title' =>
$event->getTitle()->getPrefixedText(),
+ )
+ );
+ return false;
+ }
+ return true;
+ }
+ );
+ }
+
+ /**
+ * Marks all queued notifications as read.
+ * Satisfies DeferrableUpdate interface
+ */
+ public function doUpdate() {
+ $events = $this->filterEventsWithTitleDbLag();
+
+ $eventIds = array_map(
+ function ( EchoEvent $event ) {
+ return $event->getId();
+ },
+ $events
+ );
+
+ EchoModerationController::moderate( $eventIds, true );
+ $this->events = array();
+ }
+}
diff --git a/includes/DeferredMarkAsReadUpdate.php
b/includes/DeferredMarkAsReadUpdate.php
deleted file mode 100644
index decde62..0000000
--- a/includes/DeferredMarkAsReadUpdate.php
+++ /dev/null
@@ -1,52 +0,0 @@
-<?php
-
-/**
- * Mark event notifications as read at the end of a request. Used to queue up
- * individual events to mark due to formatting failures or other uses.
- */
-class EchoDeferredMarkAsReadUpdate implements DeferrableUpdate {
- /**
- * @var array
- */
- protected $events = array();
-
- /**
- * @param EchoEvent $event
- * @param 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();
- } else {
- $this->events[$uid] = array(
- 'user' => $user,
- 'eventIds' => array( $event->getId() ),
- );
- }
- }
-
- /**
- * Mark's all queue'd notifications as read.
- * Satisfies DeferrableUpdate interface
- */
- public function doUpdate() {
- foreach ( $this->events as $data ) {
- MWEchoNotifUser::newFromUser( $data['user']
)->markRead( $data['eventIds'] );
- }
- $this->events = array();
- }
-}
diff --git a/includes/controller/NotificationController.php
b/includes/controller/NotificationController.php
index 918a943..a6fb3cc 100644
--- a/includes/controller/NotificationController.php
+++ b/includes/controller/NotificationController.php
@@ -382,21 +382,6 @@
}
/**
- * Event has failed to format for the given user. Mark it as read so
- * we do not continue to notify them about this broken event.
- *
- * @param EchoEvent $event
- * @param User $user
- */
- protected static function failFormatting( EchoEvent $event, $user ) {
- // 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 APIs
- EchoDeferredMarkAsReadUpdate::add( $event, $user );
- }
-
- /**
* INTERNAL. Must be public to be callable by the php error handling
methods.
*
* Converts E_RECOVERABLE_ERROR, such as passing null to a method
expecting
diff --git a/includes/mapper/EventMapper.php b/includes/mapper/EventMapper.php
index 8a27e5f..c231795 100644
--- a/includes/mapper/EventMapper.php
+++ b/includes/mapper/EventMapper.php
@@ -175,6 +175,52 @@
}
/**
+ * Fetch events associated with a page
+ *
+ * @param int $pageId
+ * @return EchoEvent[] Events
+ */
+ public function fetchByPage( $pageId ) {
+ $events = array();
+
+ $dbr = $this->dbFactory->getEchoDb( DB_SLAVE );
+ $res = $dbr->select(
+ array( 'echo_event', 'echo_target_page' ),
+ array( 'echo_event.*' ),
+ array(
+ 'etp_page' => $pageId
+ ),
+ __METHOD__,
+ array( 'GROUP BY' => 'etp_event' ),
+ array( 'echo_target_page' => array( 'JOIN',
'event_id=etp_event' ) )
+ );
+ if ( $res ) {
+ foreach ( $res as $row ) {
+ $events[] = EchoEvent::newFromRow( $row );
+ }
+ }
+
+ return $events;
+ }
+
+ /**
+ * Fetch event IDs associated with a page
+ *
+ * @param int $pageId
+ * @return int[] Event IDs
+ */
+ public function fetchIdsByPage( $pageId ) {
+ $events = $this->fetchByPage( $pageId );
+ $eventIds = array_map(
+ function ( EchoEvent $event ) {
+ return $event->getId();
+ },
+ $events
+ );
+ return $eventIds;
+ }
+
+ /**
* Fetch events unread by a user and associated with a page
*
* @param User $user
diff --git a/includes/mapper/NotificationMapper.php
b/includes/mapper/NotificationMapper.php
index e128456..6cc3064 100644
--- a/includes/mapper/NotificationMapper.php
+++ b/includes/mapper/NotificationMapper.php
@@ -392,7 +392,7 @@
$res = $dbr->select(
array( 'echo_notification' ),
- array( 'userId' => 'notification_user' ),
+ array( 'userId' => 'DISTINCT notification_user' ),
array(
'notification_event' => $eventIds
),
diff --git a/includes/model/Event.php b/includes/model/Event.php
index 325d8ad..8641d70 100644
--- a/includes/model/Event.php
+++ b/includes/model/Event.php
@@ -465,9 +465,10 @@
}
/**
- * @return Title|null
+ * @param bool $fromMaster
+ * @return null|Title
*/
- public function getTitle() {
+ public function getTitle( $fromMaster = false ) {
if ( $this->title ) {
return $this->title;
} elseif ( $this->pageId ) {
@@ -477,7 +478,7 @@
return $this->title = $title;
}
- return $this->title = Title::newFromID( $this->pageId );
+ return $this->title = Title::newFromID( $this->pageId,
$fromMaster ? Title::GAID_FOR_UPDATE : 0 );
} elseif ( isset( $this->extra['page_title'],
$this->extra['page_namespace'] ) ) {
return $this->title = Title::makeTitleSafe(
$this->extra['page_namespace'],
--
To view, visit https://gerrit.wikimedia.org/r/301382
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Idefe78408fd584c13aaa9174cee3055539d92848
Gerrit-PatchSet: 9
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Sbisson <sbis...@wikimedia.org>
Gerrit-Reviewer: Catrope <roan.katt...@gmail.com>
Gerrit-Reviewer: Sbisson <sbis...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
