Make seen/unseen badge more consistent across wikis

- Add a 'hasUnseen' data to the xwiki bundle so the badge can
  consider its value when changing its color even without the
  bundle being opened.
- Check and store seenTimes from all sources that the xwiki
  has in a new JS object that the SeenTimeModel can store

Bug: T134855
Change-Id: Ifdcee88b4378cdc7acb4ae5c0cbc60b76339757e
This commit is contained in:
Moriel Schottlender 2016-09-06 10:38:01 -07:00
parent da4f8f5db9
commit 00e0b9f45d
10 changed files with 281 additions and 39 deletions

View file

@ -851,14 +851,21 @@ class EchoHooks {
$msgNotificationTimestamp = $notifUser->getLastUnreadMessageTime();
$alertNotificationTimestamp = $notifUser->getLastUnreadAlertTime();
$seenAlertTime = EchoSeenTime::newFromUser( $user )->getTime( 'alert', /*flags*/ 0, TS_ISO_8601 );
$seenMsgTime = EchoSeenTime::newFromUser( $user )->getTime( 'message', /*flags*/ 0, TS_ISO_8601 );
// The variable wgEchoSeenTime is being replaced, but we should keep it around
// as a backup for at least one deployment cycle.
$seenTimeForUser = EchoSeenTime::newFromUser( $user );
$seenAlertTime = $seenTimeForUser->getTime( 'alert', /*flags*/ 0, TS_ISO_8601 );
$seenMsgTime = $seenTimeForUser->getTime( 'message', /*flags*/ 0, TS_ISO_8601 );
// Output local seen time (backwards compatibility)
$sk->getOutput()->addJsConfigVars( 'wgEchoSeenTime', array(
'alert' => $seenAlertTime,
'notice' => $seenMsgTime,
) );
// Output seen time for all sources; this is the more robust
// variable that the UI uses instead of the old (local-only) wgEchoSeenTime
$sk->getOutput()->addJsConfigVars( 'wgEchoSeenTimeSources', $notifUser->getSeenTimeForAllSources() );
if (
$wgEchoShowFooterNotice &&
!$user->getOption( 'echo-dismiss-special-page-invitation' )
@ -882,7 +889,7 @@ class EchoHooks {
if (
$msgCount != 0 && // no unread notifications
$msgNotificationTimestamp !== false && // should already always be false if count === 0
( $seenMsgTime === null || $seenMsgTime < $msgNotificationTimestamp->getTimestamp( TS_ISO_8601 ) ) // there are no unseen notifications
( $notifUser->hasUnseenNotificationsAnywhere( EchoAttributeManager::MESSAGE ) ) // there are unseen notifications
) {
$msgLinkClasses[] = 'mw-echo-unseen-notifications';
$hasUnseen = true;
@ -893,7 +900,7 @@ class EchoHooks {
if (
$alertCount != 0 && // no unread notifications
$alertNotificationTimestamp !== false && // should already always be false if count === 0
( $seenAlertTime === null || $seenAlertTime < $alertNotificationTimestamp->getTimestamp( TS_ISO_8601 ) ) // all notifications have already been seen
( $notifUser->hasUnseenNotificationsAnywhere( EchoAttributeManager::ALERT ) ) // there are unseen notifications
) {
$alertLinkClasses[] = 'mw-echo-unseen-notifications';
$hasUnseen = true;
@ -1039,8 +1046,8 @@ class EchoHooks {
if ( $lastUpdate !== false ) {
$modifiedTimes['notifications-global'] = $lastUpdate;
}
$modifiedTimes['notifications-seen-alert'] = EchoSeenTime::newFromUser( $user )->getTime( 'alert' );
$modifiedTimes['notifications-seen-message'] = EchoSeenTime::newFromUser( $user )->getTime( 'message' );
$modifiedTimes[ 'notifications-seen' ] = $notifUser->getGlobalMaxSeenTime();
}
}

View file

@ -50,6 +50,11 @@ class MWEchoNotifUser {
*/
private $mForeignData = null;
/**
* @var EchoSeenTime
*/
private $seenTime = null;
// The max notification count shown in badge
// The max number shown in bundled message, eg, <user> and 99+ others <action>.
@ -73,13 +78,15 @@ class MWEchoNotifUser {
BagOStuff $cache,
EchoUserNotificationGateway $userNotifGateway,
EchoNotificationMapper $notifMapper,
EchoTargetPageMapper $targetPageMapper
EchoTargetPageMapper $targetPageMapper,
EchoSeenTime $seenTime
) {
$this->mUser = $user;
$this->userNotifGateway = $userNotifGateway;
$this->cache = $cache;
$this->notifMapper = $notifMapper;
$this->targetPageMapper = $targetPageMapper;
$this->seenTime = $seenTime;
}
/**
@ -98,7 +105,8 @@ class MWEchoNotifUser {
ObjectCache::getMainStashInstance(),
new EchoUserNotificationGateway( $user, MWEchoDbFactory::newFromDefault() ),
new EchoNotificationMapper(),
new EchoTargetPageMapper()
new EchoTargetPageMapper(),
EchoSeenTime::newFromUser( $user )
);
}
@ -458,6 +466,138 @@ class MWEchoNotifUser {
return $res;
}
/**
* Check whether a wiki has unseen notifications.
*
* @param string $section Notification section
* @param string $wiki Wiki ID. If not given, falls back to local wiki
* @return boolean There are unseen notifications in the given wiki
*/
public function hasUnseenNotificationsOnWiki( $section = EchoAttributeManager::ALL, $wiki = null ) {
if ( $wiki === wfWikiID() || $wiki === null ) {
// Local
$latestUnreadTimeTSObj = $this->getLastUnreadNotificationTime(
true,
DB_SLAVE,
$section,
false
);
} else {
// Foreign
$latestUnreadTimeTSObj = $this->getForeignTimestamp(
$section,
$wiki
);
}
$seenTime = $this->seenTime->getTime(
$section,
0/* flags */,
TS_UNIX,
$wiki
);
if ( $latestUnreadTimeTSObj === false ) {
return false;
}
$latestUnreadTime = $latestUnreadTimeTSObj->getTimestamp( TS_UNIX );
return $latestUnreadTime > $seenTime;
}
/**
* Check whether the user has unseen notifications on any wiki
*
* @param string $section Notification section
* @param boolean $onlyForeign Check only in foreign sources
* @return boolean There are unseen notifications in the given wiki
*/
public function hasUnseenNotificationsAnywhere( $section = EchoAttributeManager::ALL, $onlyForeign = false ) {
// Check if there are unseen notifications in foreign wikis
$wikis = $this->getForeignNotifications()->getWikis( $section );
foreach ( $wikis as $wiki ) {
if ( $this->hasUnseenNotificationsOnWiki( $section, $wiki ) ) {
return true;
}
}
if ( $onlyForeign ) {
return false;
}
// Finally check if there are unseen notifications locally
return $this->hasUnseenNotificationsOnWiki( $section, wfWikiID() );
}
/**
* Get a structured seen time object for a requested source.
* This is a helper function of getting seen time for all
* sources.
*
* @param string $wiki Wiki ID
* @param string $section Notification section
* @param string $format Timestamp format; defaults to ISO 8601
* @return array Seen time values for sources
*/
protected function getSeenTimeForSource( $wiki = null, $section = EchoAttributeManager::ALL, $format = TS_ISO_8601 ) {
$wiki = $wiki === null ?
wfWikiID() : $wiki;
if ( $section === EchoAttributeManager::ALL ) {
return array(
'alert' => $this->seenTime->getTime( 'alert', /*flags*/ 0, $format, $wiki ),
'message' => $this->seenTime->getTime( 'message', /*flags*/ 0, $format, $wiki ),
);
} else {
$response = array();
$response[ $section ] = $this->seenTime->getTime( $section, /*flags*/ 0, $format, $wiki );
}
}
/**
* Output the seen time value of the local and all foreign sources
* that exist for this user.
*
* @param string $section Notification section
* @param string $format Timestamp format; defaults to ISO 8601
* @return array Seen time values for sources
*/
public function getSeenTimeForAllSources( $section = EchoAttributeManager::ALL, $format = TS_ISO_8601 ) {
$seenTime = array();
// Check if there are unread notifications in foreign wikis
$wikis = $this->getForeignNotifications()->getWikis( $section );
// Add local seen time
array_push( $wikis, wfWikiID() );
foreach ( $wikis as $wiki ) {
$seenTime[ $wiki ] = $this->getSeenTimeForSource( $wiki, $section, $format );
}
return $seenTime;
}
/**
* Get the global seen time - the maximum seen time from all available sources
*
* @param string $section Notification section
* @param string $format Timestamp format; defaults to ISO 8601
* @return int Maximum seen time over all available sources
*/
public function getGlobalSeenTime( $section = EchoAttributeManager::ALL, $format = TS_ISO_8601 ) {
// Get all available seenTime in unix so it is easiest to compare
$seenTime = $this->getSeenTimeForAllSources( $section, TS_UNIX );
$max = max( array_map( function ( $data ) use ( $section ) {
if ( $section === EchoAttributeManager::ALL ) {
return max( $data );
}
return $data[ $section ];
}, $seenTime ) );
// Return the max in the given format
return wfTimestamp( $max, $format );
}
/**
* Recalculates the number of notifications that a user has.
* @param $dbSource int use master or slave database to pull count
@ -756,7 +896,7 @@ class MWEchoNotifUser {
return $count;
}
protected function getForeignTimestamp( $section = EchoAttributeManager::ALL ) {
protected function getForeignTimestamp( $section = EchoAttributeManager::ALL, $wikiId = null ) {
global $wgEchoSectionTransition, $wgEchoBundleTransition;
if (
@ -769,7 +909,10 @@ class MWEchoNotifUser {
) {
$foreignTime = false;
$foreignData = $this->getForeignData();
foreach ( $foreignData as $data ) {
foreach ( $foreignData as $wiki => $data ) {
if ( $wikiId && $wiki !== $wikiId ) {
continue;
}
if ( $section === EchoAttributeManager::ALL ) {
foreach ( $data as $subData ) {
if ( isset( $subData['timestamp'] ) ) {
@ -789,7 +932,11 @@ class MWEchoNotifUser {
}
}
} else {
$foreignTime = $this->getForeignNotifications()->getTimestamp( $section );
if ( !$wikiId ) {
$foreignTime = $this->getForeignNotifications()->getTimestamp( $section );
} else {
$foreignTime = $this->getForeignNotifications()->getWikiTimestamp( $wikiId, $section );
}
}
return $foreignTime;
}

View file

@ -41,12 +41,12 @@ class EchoSeenTime {
private static function cache() {
static $c = null;
// Use db-replicated for persistent storage, and
// Use main stash for persistent storage, and
// wrap it with CachedBagOStuff for an in-process
// cache. (T144534)
if ( $c === null ) {
$c = new CachedBagOStuff(
ObjectCache::getInstance( 'db-replicated' )
ObjectCache::getMainStashInstance()
);
}
@ -59,20 +59,23 @@ class EchoSeenTime {
* @param int $format Format to return time in, defaults to TS_MW
* @return string|bool Timestamp in specified format, or false if no stored time
*/
public function getTime( $type = 'all', $flags = 0, $format = TS_MW ) {
public function getTime( $type = 'all', $flags = 0, $format = TS_MW, $sourceWiki = null ) {
$sourceWiki = $sourceWiki === null ? wfWikiID() : $sourceWiki;
list( $db, $prefix ) = wfSplitWikiID( $sourceWiki );
$vals = array();
if ( $type === 'all' ) {
foreach ( self::$allowedTypes as $allowed ) {
// Use TS_MW, then convert later, so max works properly for
// all formats.
$vals[] = $this->getTime( $allowed, $flags, TS_MW );
$vals[] = $this->getTime( $allowed, $flags, TS_MW, $sourceWiki );
}
return wfTimestamp( $format, min( $vals ) );
}
if ( $this->validateType( $type ) ) {
$key = wfMemcKey( 'echo', 'seen', $type, 'time', $this->user->getId() );
$key = wfForeignMemcKey( $db, $prefix, 'echo', 'seen', $type, 'time', $this->user->getId() );
$cas = 0; // Unused, but we have to pass something by reference
$data = self::cache()->get( $key, $cas, $flags );
if ( $data === false ) {
@ -80,13 +83,14 @@ class EchoSeenTime {
$data = $this->user->getOption( 'echo-seen-time', false );
}
}
if ( $data !== false ) {
$formattedData = wfTimestamp( $format, $data );
} else {
$formattedData = $data;
if ( $data === false ) {
// There is still no time set, so set time to 'now'
$data = wfTimestampNow();
$this->setTime( $data, $type, $sourceWiki );
}
return $formattedData;
return wfTimestamp( $format, $data );
}
/**
@ -95,14 +99,17 @@ class EchoSeenTime {
* @param string $time Time, in TS_MW format
* @param string $type Type of seen time to set
*/
public function setTime( $time, $type = 'all' ) {
public function setTime( $time, $type = 'all', $sourceWiki = null ) {
$sourceWiki = $sourceWiki === null ? wfWikiID() : $sourceWiki;
list( $db, $prefix ) = wfSplitWikiID( $sourceWiki );
if ( $type === 'all' ) {
foreach ( self::$allowedTypes as $allowed ) {
$this->setTime( $time, $allowed );
}
} else {
if ( $this->validateType( $type ) ) {
$key = wfMemcKey( 'echo', 'seen', $type, 'time', $this->user->getId() );
$key = wfForeignMemcKey( $db, $prefix, 'echo', 'seen', $type, 'time', $this->user->getId() );
return self::cache()->set( $key, $time );
}

View file

@ -442,9 +442,12 @@ class ApiEchoNotifications extends ApiCrossWikiBase {
$notif = EchoNotification::newFromRow( $row );
$output = EchoDataOutputFormatter::formatOutput( $notif, $format, $user, $this->getLanguage() );
$notifUser = MWEchoNotifUser::newFromUser( $user );
$hasUnseen = $notifUser->hasUnseenNotificationsAnywhere( $section, true );
// Add cross-wiki-specific data
$output['section'] = $section ?: 'all';
$output['count'] = $count;
$output['hasUnseen'] = $hasUnseen ? 1 : 0;
$output['sources'] = EchoForeignNotifications::getApiEndpoints( $wikis );
// Add timestamp information
foreach ( $output['sources'] as $wiki => &$data ) {

View file

@ -303,6 +303,10 @@
// x-wiki notification multi-group
// We need to request a new list model
newNotifData.name = 'xwiki';
// Check if the xwiki item has any unseen notifications
// before we opened it at all.
newNotifData.hasUnseen = notifData.hasUnseen;
allModels.xwiki = foreignListModel = new mw.echo.dm.CrossWikiNotificationItem( notifData.id, newNotifData );
foreignListModel.setForeign( true );
@ -566,6 +570,7 @@
// Add items
listModel.setItems( items );
}
xwikiModel.setPopulated( true );
},
function ( errCode, errObj ) {
return {

View file

@ -11,15 +11,21 @@
* @param {Object} [config] Configuration object
* @cfg {number} count The initial anticipated count of notifications through all
* of the sources.
* @cfg {number} hasUnseen Whether the bundle had any unseen notifications originally
* in any source
*/
mw.echo.dm.CrossWikiNotificationItem = function MwEchoDmCrossWikiNotificationItem( id, config ) {
config = config || {};
mw.echo.dm.CrossWikiNotificationItem.parent.call( this, id, config );
this.originallyUnseen = !!config.hasUnseen;
this.populated = false;
this.foreign = true;
this.source = null;
this.count = config.count || 0;
this.seen = !this.originallyUnseen;
this.list = new mw.echo.dm.NotificationGroupsList();
@ -105,6 +111,18 @@
var i, j, items,
sourceLists = this.getList().getItems();
// If the model is not yet populated, we must fall back
// to what the server told us about the state of any unseen
// notifications inside the bundle
if ( !this.populated ) {
return this.originallyUnseen;
}
// In this case, we already opened the bundle; if the check
// for 'hasUnseen' runs now, we should check with the actual
// items (whose status may have changed, for example if we
// re-expand the bundle without reloading the page or the
// popup) so we get the correct response
for ( i = 0; i < sourceLists.length; i++ ) {
items = sourceLists[ i ].getItems();
for ( j = 0; j < items.length; j++ ) {
@ -117,6 +135,15 @@
return false;
};
/**
* Set the populated state of this item
*
* @param {boolean} isPopulated The item is populated
*/
mw.echo.dm.CrossWikiNotificationItem.prototype.setPopulated = function ( isPopulated ) {
this.populated = isPopulated;
};
/**
* Get all items in the cross wiki notification bundle
*

View file

@ -142,6 +142,26 @@
return this.notificationModels;
};
/**
* Check if all notification models are empty.
* This is either because we haven't populated them yet, or because
* there are 0 notifications for this user at all.
*
* @return {boolean} All notification models are empty
*/
mw.echo.dm.ModelManager.prototype.areAllNotificationModelsEmpty = function () {
var model,
models = this.getAllNotificationModels();
for ( model in models ) {
if ( !models[ model ].isEmpty() ) {
return false;
}
}
return true;
};
/**
* Set the models in the manager.
*

View file

@ -7,7 +7,7 @@
* that this model handles
*/
mw.echo.dm.SeenTimeModel = function MwEchoSeenTimeModel( config ) {
var originalSeenTime;
var originalSeenTime, originalSeenTimeForAll;
config = config || {};
@ -20,13 +20,24 @@
}
originalSeenTime = mw.config.get( 'wgEchoSeenTime' ) || {};
originalSeenTimeForAll = mw.config.get( 'wgEchoSeenTimeSources' );
this.seenTime = {
local: {
alert: originalSeenTime.alert,
message: originalSeenTime.notice
}
};
if ( originalSeenTimeForAll ) {
// Replace local wiki source with the 'local' tag
originalSeenTimeForAll.local = originalSeenTimeForAll[ mw.config.get( 'wgDBname' ) ];
delete originalSeenTimeForAll[ mw.config.get( 'wgDBname' ) ];
// Store for all sources
this.seenTime = originalSeenTimeForAll;
} else {
// Backwards compatibility
this.seenTime = {
local: {
alert: originalSeenTime.alert,
message: originalSeenTime.notice
}
};
}
};
/* Initialization */

View file

@ -252,7 +252,16 @@
* Respond to SeenTime model update event
*/
mw.echo.ui.NotificationBadgeWidget.prototype.onSeenTimeModelUpdate = function () {
this.updateBadgeSeenState( this.manager.hasUnseenInSource( 'local' ) );
// SeenTimeModel may get updated with foreign source values before we
// have populated the models. In that case, a check of 'hasUnseenInAnyModel'
// will return false, because there are no items at all (and hence no
// unseen items) -- but that answer might be untrue.
//
// The state of the badge should only be updated if we already have a
// non-empty model
if ( !this.manager.areAllNotificationModelsEmpty() ) {
this.updateBadgeSeenState( this.manager.hasUnseenInAnyModel() );
}
};
/**
@ -261,7 +270,7 @@
* @param {boolean} hasUnseen There are unseen notifications
*/
mw.echo.ui.NotificationBadgeWidget.prototype.updateBadgeSeenState = function ( hasUnseen ) {
hasUnseen = hasUnseen === undefined ? this.manager.hasUnseenInSource( 'local' ) : !!hasUnseen;
hasUnseen = hasUnseen === undefined ? this.manager.hasUnseenInAnyModel() : !!hasUnseen;
this.badgeButton.setFlags( { unseen: !!hasUnseen } );
};

View file

@ -85,7 +85,8 @@ class MWEchoNotifUserTest extends MediaWikiTestCase {
$this->cache,
$this->mockEchoUserNotificationGateway( array( 'markRead' => true ) ),
$this->mockEchoNotificationMapper(),
$this->mockEchoTargetPageMapper()
$this->mockEchoTargetPageMapper(),
EchoSeenTime::newFromUser( User::newFromId( 2 ) )
);
$this->assertFalse( $notifUser->markRead( array() ) );
$this->assertTrue( $notifUser->markRead( array( 1 ) ) );
@ -95,7 +96,8 @@ class MWEchoNotifUserTest extends MediaWikiTestCase {
$this->cache,
$this->mockEchoUserNotificationGateway( array( 'markRead' => false ) ),
$this->mockEchoNotificationMapper(),
$this->mockEchoTargetPageMapper()
$this->mockEchoTargetPageMapper(),
EchoSeenTime::newFromUser( User::newFromId( 2 ) )
);
$this->assertFalse( $notifUser->markRead( array() ) );
$this->assertFalse( $notifUser->markRead( array( 1 ) ) );
@ -108,7 +110,8 @@ class MWEchoNotifUserTest extends MediaWikiTestCase {
$this->cache,
$this->mockEchoUserNotificationGateway( array( 'markRead' => true ) ),
$this->mockEchoNotificationMapper( array( $this->mockEchoNotification() ) ),
$this->mockEchoTargetPageMapper()
$this->mockEchoTargetPageMapper(),
EchoSeenTime::newFromUser( User::newFromId( 2 ) )
);
$this->assertTrue( $notifUser->markAllRead() );
@ -118,7 +121,8 @@ class MWEchoNotifUserTest extends MediaWikiTestCase {
$this->cache,
$this->mockEchoUserNotificationGateway( array( 'markRead' => false ) ),
$this->mockEchoNotificationMapper( array( $this->mockEchoNotification() ) ),
$this->mockEchoTargetPageMapper()
$this->mockEchoTargetPageMapper(),
EchoSeenTime::newFromUser( User::newFromId( 2 ) )
);
$this->assertFalse( $notifUser->markAllRead() );
@ -128,7 +132,8 @@ class MWEchoNotifUserTest extends MediaWikiTestCase {
$this->cache,
$this->mockEchoUserNotificationGateway( array( 'markRead' => true ) ),
$this->mockEchoNotificationMapper(),
$this->mockEchoTargetPageMapper()
$this->mockEchoTargetPageMapper(),
EchoSeenTime::newFromUser( User::newFromId( 2 ) )
);
$this->assertFalse( $notifUser->markAllRead() );
@ -138,7 +143,8 @@ class MWEchoNotifUserTest extends MediaWikiTestCase {
$this->cache,
$this->mockEchoUserNotificationGateway( array( 'markRead' => false ) ),
$this->mockEchoNotificationMapper(),
$this->mockEchoTargetPageMapper()
$this->mockEchoTargetPageMapper(),
EchoSeenTime::newFromUser( User::newFromId( 2 ) )
);
$this->assertFalse( $notifUser->markAllRead() );
}