wikimedia/mediawiki-extensions-Echo
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/Echo synced 2024-11-30 18:45:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "Make seen/unseen badge more consistent across wikis"

Browse source 
Has bugs, and will likely cause deployment problems.
This'll need to be reverted in wmf.19 at least
until we fix it up.

This reverts commit 00e0b9f45d.

Change-Id: Ia9d220ebcb607f96dee6bc856755305ed8501fcc
This commit is contained in:
Catrope 2016-09-13 20:00:35 +00:00 committed by Moriel Schottlender
parent 86eb821204
commit bba984c002
10 changed files with 45 additions and 283 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
Hooks.php
View file

@ -858,21 +858,14 @@ class EchoHooks {
$msgNotificationTimestamp = $notifUser->getLastUnreadMessageTime();
$alertNotificationTimestamp = $notifUser->getLastUnreadAlertTime();
// 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)
$seenAlertTime = EchoSeenTime::newFromUser( $user )->getTime( 'alert', /*flags*/ 0, TS_ISO_8601 );
$seenMsgTime = EchoSeenTime::newFromUser( $user )->getTime( 'message', /*flags*/ 0, TS_ISO_8601 );
$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' )
@ -896,7 +889,7 @@ class EchoHooks {
if (
$msgCount != 0 && // no unread notifications
$msgNotificationTimestamp !== false && // should already always be false if count === 0
( $notifUser->hasUnseenNotificationsAnywhere( EchoAttributeManager::MESSAGE ) ) // there are unseen notifications
( $seenMsgTime === null || $seenMsgTime < $msgNotificationTimestamp->getTimestamp( TS_ISO_8601 ) ) // there are no unseen notifications
) {
$msgLinkClasses[] = 'mw-echo-unseen-notifications';
$hasUnseen = true;
@ -907,7 +900,7 @@ class EchoHooks {
if (
$alertCount != 0 && // no unread notifications
$alertNotificationTimestamp !== false && // should already always be false if count === 0
( $notifUser->hasUnseenNotificationsAnywhere( EchoAttributeManager::ALERT ) ) // there are unseen notifications
( $seenAlertTime === null || $seenAlertTime < $alertNotificationTimestamp->getTimestamp( TS_ISO_8601 ) ) // all notifications have already been seen
) {
$alertLinkClasses[] = 'mw-echo-unseen-notifications';
$hasUnseen = true;
@ -1054,7 +1047,8 @@ class EchoHooks {
$modifiedTimes['notifications-global'] = $lastUpdate;
}
$modifiedTimes[ 'notifications-seen' ] = $notifUser->getGlobalSeenTime();
$modifiedTimes['notifications-seen-alert'] = EchoSeenTime::newFromUser( $user )->getTime( 'alert' );
$modifiedTimes['notifications-seen-message'] = EchoSeenTime::newFromUser( $user )->getTime( 'message' );
}
}

157
includes/NotifUser.php
View file

@ -50,11 +50,6 @@ 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>.
@ -78,15 +73,13 @@ class MWEchoNotifUser {
BagOStuff $cache,
EchoUserNotificationGateway $userNotifGateway,
EchoNotificationMapper $notifMapper,
EchoTargetPageMapper $targetPageMapper,
EchoSeenTime $seenTime
EchoTargetPageMapper $targetPageMapper
) {
$this->mUser = $user;
$this->userNotifGateway = $userNotifGateway;
$this->cache = $cache;
$this->notifMapper = $notifMapper;
$this->targetPageMapper = $targetPageMapper;
$this->seenTime = $seenTime;
}
/**
@ -105,8 +98,7 @@ class MWEchoNotifUser {
ObjectCache::getMainStashInstance(),
new EchoUserNotificationGateway( $user, MWEchoDbFactory::newFromDefault() ),
new EchoNotificationMapper(),
new EchoTargetPageMapper(),
EchoSeenTime::newFromUser( $user )
new EchoTargetPageMapper()
);
}
@ -466,138 +458,6 @@ 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
@ -897,7 +757,7 @@ class MWEchoNotifUser {
return self::capNotificationCount( $count );
}
protected function getForeignTimestamp( $section = EchoAttributeManager::ALL, $wikiId = null ) {
protected function getForeignTimestamp( $section = EchoAttributeManager::ALL ) {
global $wgEchoSectionTransition, $wgEchoBundleTransition;
if (
@ -910,10 +770,7 @@ class MWEchoNotifUser {
) {
$foreignTime = false;
$foreignData = $this->getForeignData();
foreach ( $foreignData as $wiki => $data ) {
if ( $wikiId && $wiki !== $wikiId ) {
continue;
}
foreach ( $foreignData as $data ) {
if ( $section === EchoAttributeManager::ALL ) {
foreach ( $data as $subData ) {
if ( isset( $subData['timestamp'] ) ) {
@ -933,11 +790,7 @@ class MWEchoNotifUser {
}
}
} else {
if ( !$wikiId ) {
$foreignTime = $this->getForeignNotifications()->getTimestamp( $section );
} else {
$foreignTime = $this->getForeignNotifications()->getWikiTimestamp( $wikiId, $section );
}
$foreignTime = $this->getForeignNotifications()->getTimestamp( $section );
}
return $foreignTime;
}

40
includes/SeenTime.php
View file

@ -41,12 +41,12 @@ class EchoSeenTime {
private static function cache() {
static $c = null;
// Use main stash for persistent storage, and
// Use db-replicated for persistent storage, and
// wrap it with CachedBagOStuff for an in-process
// cache. (T144534)
if ( $c === null ) {
$c = new CachedBagOStuff(
ObjectCache::getMainStashInstance()
ObjectCache::getInstance( 'db-replicated' )
);
}
@ -59,29 +59,29 @@ 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, $sourceWiki = null ) {
$sourceWiki = $sourceWiki === null ? wfWikiID() : $sourceWiki;
list( $db, $prefix ) = wfSplitWikiID( $sourceWiki );
public function getTime( $type = 'all', $flags = 0, $format = TS_MW ) {
$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, $sourceWiki );
$vals[] = $this->getTime( $allowed, $flags, TS_MW );
}
return wfTimestamp( $format, min( $vals ) );
}
if ( $this->validateType( $type ) ) {
$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 ) {
// Check if the user still has it set in their preferences
$data = $this->user->getOption( 'echo-seen-time', false );
}
if ( !$this->validateType( $type ) ) {
return false;
}
$key = wfMemcKey( '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 ) {
// Check if the user still has it set in their preferences
$data = $this->user->getOption( 'echo-seen-time', false );
}
if ( $data === false ) {
@ -89,9 +89,8 @@ class EchoSeenTime {
// We can't remember their real seen time, so reset everything to
// unseen.
$data = wfTimestamp( TS_MW, 1 );
$this->setTime( $data, $type, $sourceWiki );
$this->setTime( $data, $type );
}
return wfTimestamp( $format, $data );
}
@ -102,17 +101,14 @@ class EchoSeenTime {
* @param string $type Type of seen time to set
* @param string $sourceWiki Source wiki to set it for, defaults to current
*/
public function setTime( $time, $type = 'all', $sourceWiki = null ) {
$sourceWiki = $sourceWiki === null ? wfWikiID() : $sourceWiki;
list( $db, $prefix ) = wfSplitWikiID( $sourceWiki );
public function setTime( $time, $type = 'all' ) {
if ( $type === 'all' ) {
foreach ( self::$allowedTypes as $allowed ) {
$this->setTime( $time, $allowed );
}
} else {
if ( $this->validateType( $type ) ) {
$key = wfForeignMemcKey( $db, $prefix, 'echo', 'seen', $type, 'time', $this->user->getId() );
$key = wfMemcKey( 'echo', 'seen', $type, 'time', $this->user->getId() );
return self::cache()->set( $key, $time );
}

3
includes/api/ApiEchoNotifications.php
View file

@ -442,12 +442,9 @@ 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 ) {

5
modules/controller/mw.echo.Controller.js
View file

@ -303,10 +303,6 @@
// 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 );
@ -570,7 +566,6 @@
// Add items
listModel.setItems( items );
}
xwikiModel.setPopulated( true );
},
function ( errCode, errObj ) {
return {

27
modules/model/mw.echo.dm.CrossWikiNotificationItem.js
View file

@ -11,21 +11,15 @@
* @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();
@ -111,18 +105,6 @@
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++ ) {
@ -135,15 +117,6 @@
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
*

20
modules/model/mw.echo.dm.ModelManager.js
View file

@ -142,26 +142,6 @@
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.
*

25
modules/model/mw.echo.dm.SeenTimeModel.js
View file

@ -7,7 +7,7 @@
* that this model handles
*/
mw.echo.dm.SeenTimeModel = function MwEchoSeenTimeModel( config ) {
var originalSeenTime, originalSeenTimeForAll;
var originalSeenTime;
config = config || {};
@ -20,24 +20,13 @@
}
originalSeenTime = mw.config.get( 'wgEchoSeenTime' ) || {};
originalSeenTimeForAll = mw.config.get( 'wgEchoSeenTimeSources' );
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
}
};
}
this.seenTime = {
local: {
alert: originalSeenTime.alert,
message: originalSeenTime.notice
}
};
};
/* Initialization */

13
modules/ui/mw.echo.ui.NotificationBadgeWidget.js
View file

@ -235,16 +235,7 @@
* Respond to SeenTime model update event
*/
mw.echo.ui.NotificationBadgeWidget.prototype.onSeenTimeModelUpdate = function () {
// 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() );
}
this.updateBadgeSeenState( this.manager.hasUnseenInSource( 'local' ) );
};
/**
@ -253,7 +244,7 @@
* @param {boolean} hasUnseen There are unseen notifications
*/
mw.echo.ui.NotificationBadgeWidget.prototype.updateBadgeSeenState = function ( hasUnseen ) {
hasUnseen = hasUnseen === undefined ? this.manager.hasUnseenInAnyModel() : !!hasUnseen;
hasUnseen = hasUnseen === undefined ? this.manager.hasUnseenInSource( 'local' ) : !!hasUnseen;
this.badgeButton.setFlags( { unseen: !!hasUnseen } );
};

18
tests/phpunit/NotifUserTest.php
View file

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