From b9f2d026a66f5fee35d75dd2fc6e8d515bb54e12 Mon Sep 17 00:00:00 2001 From: Matthias Mullie Date: Mon, 25 Jan 2016 16:33:06 +0100 Subject: [PATCH] If user only has foreign messages, the messages badge should not be suppressed Bug: T124372 Change-Id: Id9699c56e9b8c6af77e74cbfc48b6e4e3464b3a5 --- Hooks.php | 2 ++ includes/NotifUser.php | 41 +++++++++++++++++++++++++++++++++++++--- includes/UnreadWikis.php | 29 +++++++++++++--------------- 3 files changed, 53 insertions(+), 19 deletions(-) diff --git a/Hooks.php b/Hooks.php index afdaacf6f..d126fd9a7 100644 --- a/Hooks.php +++ b/Hooks.php @@ -753,6 +753,8 @@ class EchoHooks { 'notifications-alert' => $alertLink, ); + // hasMessages() checks if the user has ever had (local) messages, or if they + // have any currently unread message at all (including on foreign wikis) if ( $notifUser->hasMessages() ) { $msgLink = array( 'href' => $url, diff --git a/includes/NotifUser.php b/includes/NotifUser.php index 047f1bcd7..9a8ac08d4 100644 --- a/includes/NotifUser.php +++ b/includes/NotifUser.php @@ -183,7 +183,16 @@ class MWEchoNotifUser { private function getHasMessagesKey() { global $wgEchoConfig; - return wfMemcKey( 'echo', 'user', 'had', 'messages', $this->mUser->getId(), $wgEchoConfig['version'] ); + $lookup = CentralIdLookup::factory(); + $id = $lookup->centralIdFromLocalUser( $this->mUser, CentralIdLookup::AUDIENCE_RAW ); + if ( !$id ) { + // local user + return wfMemcKey( 'echo', 'user', 'had', 'messages', $this->mUser->getId(), $wgEchoConfig['version'] ); + } else { + // central user: we don't want a per-wiki cache key: as soon as the user + // gets a message on another wiki, this cache key should be altered + return wfGlobalCacheKey( 'echo', 'user', 'had', 'messages', $id, $wgEchoConfig['version'] ); + } } /** @@ -206,13 +215,39 @@ class MWEchoNotifUser { $eventTypesToLoad = $attributeManager->getUserEnabledEventsbySections( $this->mUser, 'web', array( $section ) ); $count = count( $this->notifMapper->fetchByUser( $this->mUser, 1, 0, $eventTypesToLoad ) ); - - $result = (int)( $count > 0 ); + $result = (int)( $count > 0 ) || $this->hasForeignMessages(); $this->cache->set( $memcKey, $result, 86400 ); return (bool)$result; } + protected function hasForeignMessages() { + if ( !$this->mUser->getOption( 'echo-cross-wiki-notifications' ) ) { + return false; + } + + $uw = EchoUnreadWikis::newFromUser( $this->mUser ); + if ( $uw === false ) { + return false; + } + + $counts = $uw->getUnreadCounts(); + foreach ( $counts as $wiki => $data ) { + if ( $data[EchoAttributeManager::MESSAGE]['count'] > 0 ) { + // currently has unread notifications + return true; + } + + if ( $data[EchoAttributeManager::MESSAGE]['ts'] !== EchoUnreadWikis::DEFAULT_TS ) { + // a timestamp at which notifications were read was recorded, + // which means the user must've had messages somewhere, at some point + return true; + } + } + + return false; + } + /** * Cache the fact that the user has messages. * This is used after the user receives a message, making the system skip the actual test diff --git a/includes/UnreadWikis.php b/includes/UnreadWikis.php index 78f7bd3c2..f0243cd0c 100644 --- a/includes/UnreadWikis.php +++ b/includes/UnreadWikis.php @@ -2,9 +2,13 @@ /** * Manages what wikis a user has unread notifications on - * */ class EchoUnreadWikis { + /** + * @var string + */ + const DEFAULT_TS = '00000000000000'; + /** * @var int */ @@ -48,7 +52,7 @@ class EchoUnreadWikis { } /** - * @return array + * @return array Note that also wikis with 0 notifications and/or messages may be included */ public function getUnreadCounts() { $dbr = $this->getDB( DB_SLAVE ); @@ -69,10 +73,6 @@ class EchoUnreadWikis { $wikis = array(); foreach ( $rows as $row ) { - if ( !$row->euw_alerts && !$row->euw_messages ) { - // This shouldn't happen, but lets be safe... - continue; - } $wikis[$row->euw_wiki] = array( EchoAttributeManager::ALERT => array( 'count' => $row->euw_alerts, @@ -101,18 +101,16 @@ class EchoUnreadWikis { return; } - $defaultTS = '00000000000000'; - if ( $alertCount || $msgCount ) { $values = array( 'euw_alerts' => $alertCount, 'euw_alerts_ts' => $alertCount ? $alertTime->getTimestamp( TS_MW ) - : $defaultTS, + : static::DEFAULT_TS, 'euw_messages' => $msgCount, 'euw_messages_ts' => $msgCount ? $msgTime->getTimestamp( TS_MW ) - : $defaultTS, + : static::DEFAULT_TS, ); $dbw->upsert( 'echo_unread_wikis', @@ -125,12 +123,11 @@ class EchoUnreadWikis { __METHOD__ ); } else { - // No unread notifications, delete the row - $dbw->delete( - 'echo_unread_wikis', - array( 'euw_user' => $this->id, 'euw_wiki' => $wiki ), - __METHOD__ - ); + // Even if there are no unread notifications, don't delete the row! + // That (empty) row helps us tell the difference between "has had + // notifications but all have been seen" (0 count, non-0 timestamp) + // and "has never had a notifications before" (row with 0 count and + // 000 timestamp or no row at all) } } }