jenkins-bot has submitted this change and it was merged.

Change subject: Defer onPersonalUrls() DB writes to post-send
......................................................................


Defer onPersonalUrls() DB writes to post-send

We calculate how many messages and alerts are being marked as read, and
subtract them from the count since the database and caches won't be
updated until the end of the request.

For performance, we also get the event_type while doing the
EchoTargetPage lookup query to avoid having to query it individually
later on.

Bug: T117531
Change-Id: I0d9302adf1b4b07a4ff26a04b00d4498aa3fe7ee
---
M Hooks.php
M includes/mapper/TargetPageMapper.php
M includes/model/TargetPage.php
3 files changed, 51 insertions(+), 13 deletions(-)

Approvals:
  Catrope: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/Hooks.php b/Hooks.php
index b326795..7d51ae3 100644
--- a/Hooks.php
+++ b/Hooks.php
@@ -724,24 +724,40 @@
                        return true;
                }
 
-               // Attempt to mark a notification as read when visiting a page,
-               // ideally this should be deferred to end of request and update
-               // the notification count accordingly
-               // @Fixme - Find a better place to put this code
+               // Attempt to mark a notification as read when visiting a page
+               // @todo should this really be here?
+               $subtractAlerts = 0;
+               $subtractMessages = 0;
                if ( $title->getArticleID() ) {
                        $mapper = new EchoTargetPageMapper();
                        $targetPages = $mapper->fetchByUserPageId( $user, 
$title->getArticleID() );
                        if ( $targetPages ) {
-                               $eventIds = array_keys( $targetPages );
-                               $notifUser = MWEchoNotifUser::newFromUser( 
$user );
-                               $notifUser->markRead( $eventIds );
+                               $eventIds = array();
+                               $attribManager = 
EchoAttributeManager::newFromGlobalVars();
+                               /* @var EchoTargetPage $targetPage */
+                               foreach ( $targetPages as $id => $targetPage ) {
+                                       $section = 
$attribManager->getNotificationSection(
+                                               $targetPage->getEventType()
+                                       );
+                                       if ( $section === 
EchoAttributeManager::MESSAGE ) {
+                                               $subtractMessages += 1;
+                                       } else {
+                                               // ALERT
+                                               $subtractAlerts += 1;
+                                       }
+                                       $eventIds[] = $id;
+                               }
+                               DeferredUpdates::addCallableUpdate( function () 
use ( $user, $eventIds ) {
+                                       $notifUser = 
MWEchoNotifUser::newFromUser( $user );
+                                       $notifUser->markRead( $eventIds );
+                               } );
                        }
                }
 
                // Add a "My notifications" item to personal URLs
                $notifUser = MWEchoNotifUser::newFromUser( $user );
-               $msgCount = $notifUser->getMessageCount();
-               $alertCount = $notifUser->getAlertCount();
+               $msgCount = $notifUser->getMessageCount() - $subtractMessages;
+               $alertCount = $notifUser->getAlertCount() - $subtractAlerts;
 
                $msgNotificationTimestamp = 
$notifUser->getLastUnreadMessageTime();
                $alertNotificationTimestamp = 
$notifUser->getLastUnreadAlertTime();
diff --git a/includes/mapper/TargetPageMapper.php 
b/includes/mapper/TargetPageMapper.php
index f34803a..7cdfde9 100644
--- a/includes/mapper/TargetPageMapper.php
+++ b/includes/mapper/TargetPageMapper.php
@@ -28,13 +28,15 @@
                $dbr = $this->dbFactory->getEchoDb( DB_SLAVE );
 
                $res = $dbr->select(
-                       array( 'echo_target_page' ),
-                       self::$fields,
+                       array( 'echo_target_page', 'echo_event' ),
+                       array_merge( self::$fields, array( 'event_type' ) ),
                        array(
                                'etp_user' => $user->getId(),
                                'etp_page' => $pageId
                        ),
-                       __METHOD__
+                       __METHOD__,
+                       array(),
+                       array( 'echo_event' => array( 'JOIN', 
'etp_event=event_id' ) )
                );
                if ( $res ) {
                        $targetPages = array();
diff --git a/includes/model/TargetPage.php b/includes/model/TargetPage.php
index 1d237d7..d62437a 100644
--- a/includes/model/TargetPage.php
+++ b/includes/model/TargetPage.php
@@ -33,6 +33,11 @@
        protected $eventId;
 
        /**
+        * @var string
+        */
+       protected $eventType;
+
+       /**
         * Only allow creating instance internally
         */
        protected function __construct() {
@@ -44,7 +49,7 @@
         * @param User $user
         * @param Title $title
         * @param EchoEvent $event
-        * @return TargetPage|null
+        * @return EchoTargetPage|null
         */
        public static function create( User $user, Title $title, EchoEvent 
$event ) {
                // This only support title with a page_id
@@ -55,6 +60,7 @@
                $obj->user = $user;
                $obj->event = $event;
                $obj->eventId = $event->getId();
+               $obj->eventType = $event->getType();
                $obj->title = $title;
                $obj->pageId = $title->getArticleID();
 
@@ -83,6 +89,9 @@
                $obj->user = User::newFromId( $row->etp_user );
                $obj->pageId = $row->etp_page;
                $obj->eventId = $row->etp_event;
+               if ( isset( $row->event_type ) ) {
+                       $obj->eventType = $row->event_type;
+               }
 
                return $obj;
        }
@@ -131,6 +140,17 @@
        }
 
        /**
+        * @return string
+        */
+       public function getEventType() {
+               if ( !$this->eventType ) {
+                       $this->eventType = $this->getEvent()->getType();
+               }
+
+               return $this->eventType;
+       }
+
+       /**
         * Convert the properties to a database row
         * @return array
         */

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0d9302adf1b4b07a4ff26a04b00d4498aa3fe7ee
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

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

Reply via email to