Jdlrobson has uploaded a new change for review. https://gerrit.wikimedia.org/r/312105
Change subject: Revert "Convert 'seenTime' to a global property" ...................................................................... Revert "Convert 'seenTime' to a global property" This change breaks the notifications overlay on the mobile site. This is currently causing all of MobileFrontend's selenium jobs to fail. This reverts commit 1575e2bb7a2f686ba4fba71c19bfd70b4c18abc7. Change-Id: Ic9bd150b4ab6121abba7031447d04f3c53b1f914 --- M includes/SeenTime.php M modules/api/mw.echo.api.EchoApi.js M modules/controller/mw.echo.Controller.js M modules/model/mw.echo.dm.BundleNotificationItem.js M modules/model/mw.echo.dm.CrossWikiNotificationItem.js M modules/model/mw.echo.dm.ModelManager.js M modules/model/mw.echo.dm.NotificationItem.js M modules/model/mw.echo.dm.NotificationsList.js M modules/model/mw.echo.dm.SeenTimeModel.js M modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js M modules/ui/mw.echo.ui.NotificationBadgeWidget.js M modules/ui/mw.echo.ui.NotificationsInboxWidget.js 12 files changed, 103 insertions(+), 115 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Echo refs/changes/05/312105/1 diff --git a/includes/SeenTime.php b/includes/SeenTime.php index 687613c..4a33720 100644 --- a/includes/SeenTime.php +++ b/includes/SeenTime.php @@ -41,12 +41,12 @@ 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' ) ); } @@ -75,8 +75,9 @@ 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( $this->getMemcKey( $type ), $cas, $flags ); + $data = self::cache()->get( $key, $cas, $flags ); if ( $data === false ) { // Check if the user still has it set in their preferences @@ -107,7 +108,9 @@ } } else { if ( $this->validateType( $type ) ) { - return self::cache()->set( $this->getMemcKey( $type ), $time ); + $key = wfMemcKey( 'echo', 'seen', $type, 'time', $this->user->getId() ); + + return self::cache()->set( $key, $time ); } } } @@ -120,29 +123,5 @@ */ private function validateType( $type ) { return in_array( $type, self::$allowedTypes ); - } - - /** - * Build a memcached key. - * - * @param string $type Given notification type - * @return string Memcached key - */ - protected function getMemcKey( $type = 'all' ) { - $localKey = wfMemcKey( 'echo', 'seen', $type, 'time', $this->user->getId() ); - - if ( !$this->user->getOption( 'echo-cross-wiki-notifications' ) ) { - return $localKey; - } - - $lookup = CentralIdLookup::factory(); - $globalId = $lookup->centralIdFromLocalUser( $this->user, CentralIdLookup::AUDIENCE_RAW ); - - if ( !$globalId ) { - return $localKey; - } - - return wfGlobalCacheKey( 'echo', 'seen', $type, 'time', $globalId ); - } } diff --git a/modules/api/mw.echo.api.EchoApi.js b/modules/api/mw.echo.api.EchoApi.js index ebf4d2a..3ea96b7 100644 --- a/modules/api/mw.echo.api.EchoApi.js +++ b/modules/api/mw.echo.api.EchoApi.js @@ -271,10 +271,7 @@ }; /** - * Update the seenTime property for the given type. - * We only need to update this in a single source for the seenTime - * to be updated globally - but we will let the consumer of - * this method override the choice of which source to update. + * Update the seenTime property for the given type and source. * * @param {string} [type='alert,message'] Notification type * @param {string} [source='local'] Notification source @@ -283,7 +280,6 @@ mw.echo.api.EchoApi.prototype.updateSeenTime = function ( type, source ) { source = source || 'local'; type = type || [ 'alert', 'message' ]; - return this.network.getApiHandler( source ).updateSeenTime( type ); }; diff --git a/modules/controller/mw.echo.Controller.js b/modules/controller/mw.echo.Controller.js index 0945e64..59cb771 100644 --- a/modules/controller/mw.echo.Controller.js +++ b/modules/controller/mw.echo.Controller.js @@ -158,7 +158,8 @@ // anyways. maxSeenTime = data.seenTime.alert < data.seenTime.notice ? data.seenTime.notice : data.seenTime.alert; - controller.manager.getSeenTimeModel().setSeenTime( + controller.manager.getSeenTimeModel().setSeenTimeForSource( + currentSource, maxSeenTime ); @@ -286,7 +287,8 @@ content = notifData[ '*' ] || {}; // Set source's seenTime - controller.manager.getSeenTimeModel().setSeenTime( + controller.manager.getSeenTimeModel().setSeenTimeForSource( + 'local', controller.getTypes().length > 1 ? ( data.seenTime.alert < data.seenTime.notice ? @@ -366,6 +368,7 @@ */ mw.echo.Controller.prototype.createNotificationData = function ( apiData ) { var utcTimestamp, utcIsoMoment, + source = this.manager.getFiltersModel().getSourcePagesModel().getCurrentSource(), content = apiData[ '*' ] || {}; if ( apiData.timestamp.utciso8601 ) { @@ -385,7 +388,7 @@ read: !!apiData.read, seen: ( !!apiData.read || - utcTimestamp <= this.manager.getSeenTime() + utcTimestamp <= this.manager.getSeenTime( source ) ), timestamp: utcTimestamp, category: apiData.category, @@ -734,26 +737,65 @@ }; /** - * Update global seenTime for all sources + * Update seenTime for the given source * * @return {jQuery.Promise} A promise that is resolved when the - * seenTime was updated for all the controller's types and sources. + * seenTime was updated for all the controller's types. */ - mw.echo.Controller.prototype.updateSeenTime = function () { + mw.echo.Controller.prototype.updateSeenTime = function ( source ) { var controller = this; - return this.api.updateSeenTime( - this.getTypes(), - // For consistency, use current source, though seenTime - // will be updated globally - this.manager.getFiltersModel().getSourcePagesModel().getCurrentSource() - ) + return this.api.updateSeenTime( this.getTypes(), source ) .then( function ( time ) { - controller.manager.getSeenTimeModel().setSeenTime( time ); + controller.manager.getSeenTimeModel().setSeenTimeForSource( source, time ); } ); }; /** + * Update local seen time + * + * @return {jQuery.Promise} A promise that is resolved when the + * seenTime was updated for all given types. + */ + mw.echo.Controller.prototype.updateLocalSeenTime = function () { + return this.updateSeenTime( 'local' ); + }; + + /** + * Update seen time for all sources within a cross-wiki bundle. + * + * @return {jQuery.Promise} A promise that is resolved when the + * seenTime was updated for all available cross-wiki sources. + */ + mw.echo.Controller.prototype.updateSeenTimeForCrossWiki = function () { + var model = this.manager.getNotificationModel( 'xwiki' ), + controller = this, + promises = []; + + if ( !model ) { + // There is no xwiki notifications model + return $.Deferred().reject().promise(); + } + + model.getSourceNames().forEach( function ( source ) { + promises.push( controller.updateSeenTime( source ) ); + } ); + + return mw.echo.api.NetworkHandler.static.waitForAllPromises( promises ); + }; + /** + * Update seenTime for the currently selected source + * + * @return {jQuery.Promise} A promise that is resolved when the + * seenTime was updated for all given types. + */ + mw.echo.Controller.prototype.updateSeenTimeForCurrentSource = function () { + var currSource = this.manager.getFiltersModel().getSourcePagesModel().getCurrentSource(); + + return this.updateSeenTime( currSource ); + }; + + /** * Perform a dynamic action * * @param {Object} data Action data for the network diff --git a/modules/model/mw.echo.dm.BundleNotificationItem.js b/modules/model/mw.echo.dm.BundleNotificationItem.js index 9a28f42..62f7363 100644 --- a/modules/model/mw.echo.dm.BundleNotificationItem.js +++ b/modules/model/mw.echo.dm.BundleNotificationItem.js @@ -97,19 +97,6 @@ }; /** - * Set all notifications to seen - * - * @param {number} timestamp New seen timestamp - */ - mw.echo.dm.BundleNotificationItem.prototype.updateSeenState = function ( timestamp ) { - this.list.getItems().forEach( function ( notification ) { - notification.toggleSeen( - notification.isRead() || notification.getTimestamp() < timestamp - ); - } ); - }; - - /** * This item is a group. * This method is required for all models that are managed by the * mw.echo.dm.ModelManager. diff --git a/modules/model/mw.echo.dm.CrossWikiNotificationItem.js b/modules/model/mw.echo.dm.CrossWikiNotificationItem.js index eddf484..e7f64d0 100644 --- a/modules/model/mw.echo.dm.CrossWikiNotificationItem.js +++ b/modules/model/mw.echo.dm.CrossWikiNotificationItem.js @@ -118,21 +118,6 @@ }; /** - * Set all notifications in all groups to seen - * - * @param {number} timestamp New seen timestamp - */ - mw.echo.dm.CrossWikiNotificationItem.prototype.updateSeenState = function ( timestamp ) { - this.getList().getItems().forEach( function ( source ) { - source.getItems().forEach( function ( notification ) { - notification.toggleSeen( - notification.isRead() || notification.getTimestamp() < timestamp - ); - } ); - } ); - }; - - /** * Get all items in the cross wiki notification bundle * * @return {mw.echo.dm.NotificationItem[]} All items across all sources diff --git a/modules/model/mw.echo.dm.ModelManager.js b/modules/model/mw.echo.dm.ModelManager.js index 2cdb76a..706413d 100644 --- a/modules/model/mw.echo.dm.ModelManager.js +++ b/modules/model/mw.echo.dm.ModelManager.js @@ -107,18 +107,16 @@ /** * Respond to seen time change for a given source * + * @param {string} source Source where seen time has changed * @param {string} timestamp Seen time, as a full UTC ISO 8601 timestamp - * @fires seen */ - mw.echo.dm.ModelManager.prototype.onSeenTimeUpdate = function ( timestamp ) { - var modelId, - models = this.getAllNotificationModels(); + mw.echo.dm.ModelManager.prototype.onSeenTimeUpdate = function ( source, timestamp ) { + var notifs = this.getNotificationsBySource( source ); + notifs.forEach( function ( notification ) { + notification.toggleSeen( notification.isRead() || notification.getTimestamp() < timestamp ); + } ); - for ( modelId in models ) { - models[ modelId ].updateSeenState( timestamp ); - } - - this.emit( 'seen', timestamp ); + this.emit( 'seen', source, timestamp ); }; /** diff --git a/modules/model/mw.echo.dm.NotificationItem.js b/modules/model/mw.echo.dm.NotificationItem.js index fdd4d30..02609cc 100644 --- a/modules/model/mw.echo.dm.NotificationItem.js +++ b/modules/model/mw.echo.dm.NotificationItem.js @@ -199,12 +199,7 @@ */ mw.echo.dm.NotificationItem.prototype.toggleSeen = function ( seen ) { seen = seen !== undefined ? seen : !this.seen; - if ( - this.seen !== seen && - // Do not change the state of a read item, since its - // seen state (never 'unseen') never changes - !this.isRead() - ) { + if ( this.seen !== seen ) { this.seen = seen; this.emit( 'update' ); } diff --git a/modules/model/mw.echo.dm.NotificationsList.js b/modules/model/mw.echo.dm.NotificationsList.js index 67334f6..32e787c 100644 --- a/modules/model/mw.echo.dm.NotificationsList.js +++ b/modules/model/mw.echo.dm.NotificationsList.js @@ -254,19 +254,6 @@ }; /** - * Set all notifications to seen - * - * @param {string} timestamp New seen timestamp - */ - mw.echo.dm.NotificationsList.prototype.updateSeenState = function ( timestamp ) { - this.getItems().forEach( function ( notification ) { - notification.toggleSeen( - notification.isRead() || notification.getTimestamp() < timestamp - ); - } ); - }; - - /** * @inheritdoc */ mw.echo.dm.NotificationsList.prototype.isGroup = function () { diff --git a/modules/model/mw.echo.dm.SeenTimeModel.js b/modules/model/mw.echo.dm.SeenTimeModel.js index 073adf4..f61a5bb 100644 --- a/modules/model/mw.echo.dm.SeenTimeModel.js +++ b/modules/model/mw.echo.dm.SeenTimeModel.js @@ -7,6 +7,8 @@ * that this model handles */ mw.echo.dm.SeenTimeModel = function MwEchoSeenTimeModel( config ) { + var originalSeenTime; + config = config || {}; // Mixin constructor @@ -17,7 +19,14 @@ this.types = Array.isArray( config.types ) ? config.types : [ config.types ]; } - this.seenTime = mw.config.get( 'wgEchoSeenTime' ) || {}; + originalSeenTime = mw.config.get( 'wgEchoSeenTime' ) || {}; + + this.seenTime = { + local: { + alert: originalSeenTime.alert, + message: originalSeenTime.notice + } + }; }; /* Initialization */ @@ -29,6 +38,7 @@ /** * @event update + * @param {string} source The source that updated its seenTime * @param {string} time Seen time, as a full UTC ISO 8601 timestamp. * * Seen time has been updated for the given source @@ -37,34 +47,42 @@ /* Methods */ /** - * Get the global seenTime value + * Get the seenTime value for the source * + * @param {string} source Source name * @return {string} Seen time, as a full UTC ISO 8601 timestamp. */ - mw.echo.dm.SeenTimeModel.prototype.getSeenTime = function () { - return this.seenTime[ this.getTypes()[ 0 ] ] || 0; + mw.echo.dm.SeenTimeModel.prototype.getSeenTime = function ( source ) { + source = source || 'local'; + + return ( this.seenTime[ source ] && this.seenTime[ source ][ this.getTypes()[ 0 ] ] ) || 0; }; /** * Set the seen time value for the source * * @private + * @param {string} [source='local'] Given source * @param {string} time Seen time, as a full UTC ISO 8601 timestamp. * @fires update */ - mw.echo.dm.SeenTimeModel.prototype.setSeenTime = function ( time ) { + mw.echo.dm.SeenTimeModel.prototype.setSeenTimeForSource = function ( source, time ) { var model = this, hasChanged = false; + source = source || 'local'; + + this.seenTime[ source ] = this.seenTime[ source ] || {}; + this.getTypes().forEach( function ( type ) { - if ( model.seenTime[ type ] !== time ) { - model.seenTime[ type ] = time; + if ( model.seenTime[ source ][ type ] !== time ) { + model.seenTime[ source ][ type ] = time; hasChanged = true; } } ); if ( hasChanged ) { - this.emit( 'update', time ); + this.emit( 'update', source, time ); } }; diff --git a/modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js b/modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js index a596b44..42a95f6 100644 --- a/modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js +++ b/modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js @@ -299,6 +299,7 @@ } } ) + .then( this.controller.updateSeenTimeForCrossWiki.bind( this.controller ) ) .always( this.popPending.bind( this ) ); // Only run the fetch notifications action once diff --git a/modules/ui/mw.echo.ui.NotificationBadgeWidget.js b/modules/ui/mw.echo.ui.NotificationBadgeWidget.js index d7dcbdb..8d833b2 100644 --- a/modules/ui/mw.echo.ui.NotificationBadgeWidget.js +++ b/modules/ui/mw.echo.ui.NotificationBadgeWidget.js @@ -235,16 +235,16 @@ * Respond to SeenTime model update event */ mw.echo.ui.NotificationBadgeWidget.prototype.onSeenTimeModelUpdate = function () { - this.updateBadgeSeenState( false ); + this.updateBadgeSeenState( this.manager.hasUnseenInSource( 'local' ) ); }; /** * Update the badge style to match whether it contains unseen notifications. * - * @param {boolean} [hasUnseen=false] There are unseen notifications + * @param {boolean} hasUnseen There are unseen notifications */ mw.echo.ui.NotificationBadgeWidget.prototype.updateBadgeSeenState = function ( hasUnseen ) { - hasUnseen = hasUnseen === undefined ? false : !!hasUnseen; + hasUnseen = hasUnseen === undefined ? this.manager.hasUnseenInSource( 'local' ) : !!hasUnseen; this.badgeButton.setFlags( { unseen: !!hasUnseen } ); }; @@ -336,7 +336,7 @@ if ( widget.popup.isVisible() ) { widget.popup.clip(); // Update seen time - return widget.controller.updateSeenTime(); + return widget.controller.updateLocalSeenTime(); } }, // Failure diff --git a/modules/ui/mw.echo.ui.NotificationsInboxWidget.js b/modules/ui/mw.echo.ui.NotificationsInboxWidget.js index d46f72d..4e1df77 100644 --- a/modules/ui/mw.echo.ui.NotificationsInboxWidget.js +++ b/modules/ui/mw.echo.ui.NotificationsInboxWidget.js @@ -245,7 +245,7 @@ function () { widget.popPending(); // Update seen time - widget.controller.updateSeenTime(); + widget.controller.updateSeenTimeForCurrentSource(); }, // Failure function ( errObj ) { -- To view, visit https://gerrit.wikimedia.org/r/312105 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic9bd150b4ab6121abba7031447d04f3c53b1f914 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Echo Gerrit-Branch: master Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits