Relate read-state filter and mark read/unread action

When we are viewing a certain read state filter ('read' or 'unread')
the visibility of items should correspond to that state even when
the user marks a specific item as read/unread. That means that the
system should remove these items from view when the action is taken.

In this commit:
* The controller makes the judgment of whether to remove items when
  read/unread action is taken, based on whether a filter is set.
* We clean up the terminology of discard - no more 'remove' - to
  make sure we have consistency in the code.
* Related: The 'discard' event is now scoped within the hierarchy;
  meaning, lists emit 'discard' when an item is removed, grouplist
  emits 'discard' when a group is removed, and the manager emits
  'discard' when an entire notification model is removed. This
  means we can actually have proper hierarchy and organization with
  a single event, and not worry about clashing between the intentional
  'discard' action and the event 'remove' that is also used while
  resorting happens.
* The model manager emits a discard event when a model is removed
  so that the general list can listen to the manager and remove an
  entire batch of items if needed.
* The pagination model now updates the count for the current page
  rather than some vague notion of the last page. This is also
  updated when the controller removes items, so we can get an
  accurate count in the page for the number of notifications that
  are displayed.

Bug: T136891
Change-Id: I247c618042ef256fadf09922f7b83bd1ad361f64

modules/controller/mw.echo.Controller.js

@@ -134,7 +134,7 @@
 			}
 		)
 			.then( function ( data ) {
-				var i, notifData, newNotifData, date, itemModel, symbolicName, count,
+				var i, notifData, newNotifData, date, itemModel, symbolicName,
 					dateItemIds = {},
 					dateItems = {},
 					models = {};
@@ -189,12 +189,6 @@
 				controller.manager.setNotificationModels( models );
 
 				// Update the pagination
-				count = controller.manager.getAllNotificationCount();
-				if ( count < pagination.getItemsPerPage() ) {
-					pagination.setLastPageItemCount(
-						controller.manager.getAllNotificationCount()
-					);
-				}
 				pagination.setNextPageContinue( data.continue );
 
 				return dateItemIds;
@@ -466,7 +460,9 @@
 	 *  for the set type of this controller, in the given source.
 	 */
 	mw.echo.Controller.prototype.markItemsRead = function ( itemIds, modelName, isRead ) {
-		var model = this.manager.getNotificationModel( modelName ),
+		var items,
+			model = this.manager.getNotificationModel( modelName ),
+			readState = this.manager.getFiltersModel().getReadState(),
 			allIds = [];
 
 		itemIds = Array.isArray( itemIds ) ? itemIds : [ itemIds ];
@@ -474,11 +470,33 @@
 		// Default to true
 		isRead = isRead === undefined ? true : isRead;
 
-		model.findByIds( itemIds ).forEach( function ( notification ) {
+		items = model.findByIds( itemIds );
+
+		// If we are only looking at specific read state,
+		// then we need to make sure the items are removed
+		// from the visible list, because they no longer
+		// correspond with the chosen state filter
+		if ( readState === 'read' && !isRead ) {
+			model.discardItems( items );
+		} else if ( readState === 'unread' && isRead ) {
+			model.discardItems( items );
+			// TODO: We should also find a way to update the pagination
+			// here properly. Do we pull more items from the next page
+			// when items are cleared? Do we set some threshhold for
+			// removed items where if it is reached, we update the list
+			// to reflect the new pagination? etc.
+		}
+
+		items.forEach( function ( notification ) {
 			allIds = allIds.concat( notification.getAllIds() );
+			if ( readState === 'all' ) {
 				notification.toggleRead( isRead );
+			}
 		} );
 
+		// Update pagination count
+		this.manager.updateCurrentPageItemCount();
+
 		this.manager.getUnreadCounter().estimateChange( isRead ? -allIds.length : allIds.length );
 
 		return this.api.markItemsRead( allIds, model.getSource(), isRead ).then( this.refreshUnreadCount.bind( this ) );
@@ -505,7 +523,7 @@
 
 		itemIds = Array.isArray( itemIds ) ? itemIds : [ itemIds ];
 
-		sourceModel = xwikiModel.getList().getGroupBySource( source );
+		sourceModel = xwikiModel.getList().getGroupByName( source );
 		notifs = sourceModel.findByIds( itemIds );
 		sourceModel.discardItems( notifs );
 

modules/model/mw.echo.dm.CrossWikiNotificationItem.js

@@ -23,7 +23,7 @@
 
 		this.list = new mw.echo.dm.NotificationGroupsList();
 
-		this.list.connect( this, { remove: 'onListRemove' } );
+		this.list.connect( this, { discard: 'onListDiscard' } );
 	};
 
 	OO.inheritClass( mw.echo.dm.CrossWikiNotificationItem, mw.echo.dm.NotificationItem );
@@ -31,10 +31,10 @@
 	/* Events */
 
 	/**
-	 * @event removeSource
-	 * @param {string} name The symbolic name for the source that was removed
+	 * @event discard
+	 * @param {string} name The symbolic name for the list model that was discarded
 	 *
-	 * Source list has been removed
+	 * A sub list has been discarded
 	 */
 
 	/* Methods */
@@ -43,11 +43,10 @@
 	 * Respond to list being removed from the cross-wiki bundle.
 	 *
 	 * @param {mw.echo.dm.NotificationGroupsList} sourceModel The source model that was removed
-	 * @fires removeSource
+	 * @fires discard
 	 */
-	mw.echo.dm.CrossWikiNotificationItem.prototype.onListRemove = function ( sourceModel ) {
-		this.emit( 'removeSource', sourceModel.getName() );
-
+	mw.echo.dm.CrossWikiNotificationItem.prototype.onListDiscard = function ( sourceModel ) {
+		this.emit( 'discard', sourceModel.getName() );
 	};
 
 	/**
@@ -83,7 +82,7 @@
 	 * @return {mw.echo.dm.NotificationGroupsList} Source item
 	 */
 	mw.echo.dm.CrossWikiNotificationItem.prototype.getItemBySource = function ( sourceName ) {
-		return this.list.getGroupBySource( sourceName );
+		return this.list.getGroupByName( sourceName );
 	};
 
 	/**
@@ -129,4 +128,8 @@
 		return true;
 	};
 
+	mw.echo.dm.CrossWikiNotificationItem.prototype.isEmpty = function () {
+		return this.getList().isEmpty();
+	};
+
 } )( mediaWiki );

modules/model/mw.echo.dm.ModelManager.js

@@ -24,6 +24,7 @@
 	 * @param {Object} [config] Configuration object
 	 * @cfg {string|string[]} [type="message"] The type of the notifications in
 	 *  the models that this manager handles.
+	 * @cfg {number} [itemsPerPage=25] Number of items per page
 	 */
 	mw.echo.dm.ModelManager = function MwEchoDmModelManager( counter, config ) {
 		config = config || {};
@@ -40,7 +41,9 @@
 		this.seenTimeModel = new mw.echo.dm.SeenTimeModel( { types: this.types } );
 
 		this.notificationModels = {};
-		this.paginationModel = new mw.echo.dm.PaginationModel();
+		this.paginationModel = new mw.echo.dm.PaginationModel( {
+			itemsPerPage: config.itemsPerPage || 25
+		} );
 		this.filtersModel = new mw.echo.dm.FiltersModel( {
 			selectedSource: 'local'
 		} );
@@ -62,10 +65,10 @@
 	 */
 
 	/**
-	 * @event remove
-	 * @param {string} Removed model
+	 * @event discard
+	 * @param {string} modelId Discard model id
 	 *
-	 * A model has been removed
+	 * A model has been permanently removed
 	 */
 
 	/**
@@ -82,6 +85,14 @@
 	 * There are no more local talk page notifications
 	 */
 
+	/**
+	 * @event modelItemUpdate
+	 * @param {string} modelId Model ID
+	 * @param {mw.echo.dm.NotificationItem} item Updated item
+	 *
+	 * A specific item inside a notifications model has been updated
+	 */
+
 	/* Methods */
 
 	/**
@@ -98,6 +109,20 @@
 		this.emit( 'seen', source, timestamp );
 	};
 
+	/**
+	 * Respond to a notification model discarding items.
+	 *
+	 * @param {string} modelId Model name
+	 */
+	mw.echo.dm.ModelManager.prototype.onModelDiscardItems = function ( modelId ) {
+		var model = this.getNotificationModel( modelId );
+
+		// If the model is empty, remove it
+		if ( model.isEmpty() ) {
+			this.removeNotificationModel( modelId );
+		}
+	};
+
 	/**
 	 * Get the notifications
 	 *
@@ -118,23 +143,41 @@
 	 * }
 	 */
 	mw.echo.dm.ModelManager.prototype.setNotificationModels = function ( modelDefinitions ) {
-		var modelId,
-			localModel;
+		var modelId;
 
 		this.resetNotificationModels();
 
 		for ( modelId in modelDefinitions ) {
 			this.notificationModels[ modelId ] = modelDefinitions[ modelId ];
+			this.notificationModels[ modelId ].connect( this, {
+				discard: [ 'onModelDiscardItems', modelId ],
+				itemUpdate: [ 'onModelItemUpdate', modelId ]
+			} );
 		}
 
-		localModel = this.getNotificationModel( 'local' );
-		if ( localModel ) {
-			localModel.connect( this, { itemUpdate: 'checkLocalUnreadTalk' } );
-		}
+		// Update pagination count
+		this.updateCurrentPageItemCount();
 
 		this.emit( 'update', this.getAllNotificationModels() );
 	};
 
+	mw.echo.dm.ModelManager.prototype.onModelItemUpdate = function ( modelId, item ) {
+		var model = this.getNotificationModel( modelId );
+
+		if ( model.getSource() === 'local' ) {
+			this.checkLocalUnreadTalk();
+		}
+
+		this.emit( 'modelItemUpdate', modelId, item );
+	};
+
+	/**
+	 * Update the current page item count based on available items
+	 */
+	mw.echo.dm.ModelManager.prototype.updateCurrentPageItemCount = function () {
+		this.getPaginationModel().setCurrentPageItemCount( this.getAllNotificationCount() );
+	};
+
 	/**
 	 * Go over all the notification models and return the total number of
 	 * available notifications.
@@ -188,7 +231,8 @@
 	 */
 	mw.echo.dm.ModelManager.prototype.removeNotificationModel = function ( modelName ) {
 		delete this.notificationModels[ modelName ];
-		this.emit( 'remove', modelName );
+
+		this.emit( 'discard', modelName );
 	};
 
 	/**

modules/model/mw.echo.dm.NotificationGroupsList.js

@@ -30,32 +30,40 @@
 			}
 
 			// Fallback on Source
-			return b.getSource() - a.getSource();
+			return b.getName() - a.getName();
 		} );
 
 		this.foreign = !!config.foreign;
 		this.groups = {};
 
-		this.aggregate( { remove: 'groupRemoveItem' } );
-		this.connect( this, { groupRemoveItem: 'onGroupRemoveItem' } );
+		this.aggregate( { discard: 'groupDiscardItem' } );
+		this.connect( this, { groupDiscardItem: 'onGroupDiscardItem' } );
 	};
 
 	/* Initialization */
 	OO.inheritClass( mw.echo.dm.NotificationGroupsList, mw.echo.dm.SortedList );
 
+	/* Events */
+
+	/**
+	 * @event discard
+	 *
+	 * A group was permanently removed
+	 */
+
 	/* Methods */
 
 	/**
-	 * Handle a remove event from any list.
-	 * This means that one of the sources has removed an item.
+	 * Handle a discard event from any list.
+	 * This means that one of the sources has discarded an item.
 	 *
 	 * @param {mw.echo.dm.NotificationsList} groupList List source model for the item
 	 */
-	mw.echo.dm.NotificationGroupsList.prototype.onGroupRemoveItem = function ( groupList ) {
+	mw.echo.dm.NotificationGroupsList.prototype.onGroupDiscardItem = function ( groupList ) {
 		// Check if the list has anything at all
 		if ( groupList.isEmpty() ) {
 			// Remove it
-			this.removeItems( [ groupList ] );
+			this.removeGroup( groupList.getName() );
 		}
 	};
 
@@ -108,7 +116,7 @@
 	 * @param {mw.echo.dm.NotificationItem[]} groupItems Items to add to this group
 	 */
 	mw.echo.dm.NotificationGroupsList.prototype.addItemsToGroup = function ( groupSource, groupItems ) {
-		var group = this.getGroupBySource( groupSource );
+		var group = this.getGroupByName( groupSource );
 
 		if ( group ) {
 			group.addItems( groupItems );
@@ -118,28 +126,36 @@
 	 * Remove a group from the list. This is an easier to use alias
 	 * to 'removeItems()' method.
 	 *
-	 * @param {string} groupSource Group source name
+	 * Since this is an intentional action, we fire 'discard' event.
+	 * The main reason for this is that the event 'remove' is a general
+	 * event that is fired both when a user intends on removing an
+	 * item and also when an item is temporarily removed to be re-added
+	 * for the sake of sorting. To avoid ambiguity, we use 'discard' event.
+	 *
+	 * @param {string} groupName Group name
+	 * @fires discard
 	 */
-	mw.echo.dm.NotificationGroupsList.prototype.removeGroup = function ( groupSource ) {
-		var group = this.getGroupBySource( groupSource );
+	mw.echo.dm.NotificationGroupsList.prototype.removeGroup = function ( groupName ) {
+		var group = this.getGroupByName( groupName );
 
 		if ( group ) {
 			this.removeItems( group );
+			this.emit( 'discard', group );
 		}
 	};
 
 	/**
 	 * Get a group by its source identifier.
 	 *
-	 * @param {string} groupSource Group source
+	 * @param {string} groupName Group name
 	 * @return {mw.echo.dm.NotificationsList|null} Requested group, null if none was found.
 	 */
-	mw.echo.dm.NotificationGroupsList.prototype.getGroupBySource = function ( groupSource ) {
+	mw.echo.dm.NotificationGroupsList.prototype.getGroupByName = function ( groupName ) {
 		var i,
 			items = this.getItems();
 
 		for ( i = 0; i < items.length; i++ ) {
-			if ( items[ i ].getSource() === groupSource ) {
+			if ( items[ i ].getName() === groupName ) {
 				return items[ i ];
 			}
 		}

modules/model/mw.echo.dm.NotificationsList.js

@@ -79,6 +79,13 @@
 	 * An item in the list has been updated
 	 */
 
+	/**
+	 * @event discard
+	 * @param {mw.echo.dm.NotificationItem} item Item that was discarded
+	 *
+	 * An item was discarded
+	 */
+
 	/* Methods */
 
 	/**

modules/model/mw.echo.dm.PaginationModel.js

@@ -8,9 +8,10 @@
 	 * @constructor
 	 * @param {Object} config Configuration object
 	 * @cfg {string} [pageNext] The continue value of the next page
-	 * @cfg {number} [lastPageItemCount] The number of items that are in the
-	 *  last page.
 	 * @cfg {number} [itemsPerPage] The number of items per page
+	 * @cfg {number} [currentPageItemCount] The number of items that are in the
+	 *  current page. If not given, the initial count defaults to the total number
+	 *  of items per page.
 	 */
 	mw.echo.dm.PaginationModel = function MwEchoDmPaginationModel( config ) {
 		config = config || {};
@@ -19,8 +20,8 @@
 		OO.EventEmitter.call( this );
 
 		this.pagesContinue = [];
-		this.lastPageItemCount = this.lastPageItemCount || 0;
 		this.itemsPerPage = this.itemsPerPage || 25;
+		this.currentPageItemCount = config.currentPageItemCount || this.itemsPerPage;
 
 		// Set initial page
 		this.currPageIndex = 0;
@@ -55,7 +56,7 @@
 	mw.echo.dm.PaginationModel.prototype.reset = function () {
 		this.pagesContinue = [];
 		this.currPageIndex = 0;
-		this.lastPageItemCount = 0;
+		this.currentPageItemCount = this.itemsPerPage;
 
 		this.emit( 'update' );
 	};
@@ -180,21 +181,25 @@
 	};
 
 	/**
-	 * Set the number of items in the last page
+	 * Set the number of items in the current page
 	 *
 	 * @param {number} count Number of items
+	 * @fires update
 	 */
-	mw.echo.dm.PaginationModel.prototype.setLastPageItemCount = function ( count ) {
-		this.lastPageItemCount = count;
+	mw.echo.dm.PaginationModel.prototype.setCurrentPageItemCount = function ( count ) {
+		if ( this.currentPageItemCount !== count ) {
+			this.currentPageItemCount = count;
+			this.emit( 'update' );
+		}
 	};
 
 	/**
-	 * Get the number of items in the last page
+	 * Get the number of items in the current page
 	 *
 	 * @return {number} Number of items
 	 */
-	mw.echo.dm.PaginationModel.prototype.getLastPageItemCount = function () {
-		return this.lastPageItemCount;
+	mw.echo.dm.PaginationModel.prototype.getCurrentPageItemCount = function () {
+		return this.currentPageItemCount;
 	};
 
 	/**

modules/special/ext.echo.special.js

@@ -8,13 +8,13 @@
 			$content = $( '#mw-content-text' ),
 			echoApi = new mw.echo.api.EchoApi( { limit: limitNotifications, bundle: false } ),
 			unreadCounter = new mw.echo.dm.UnreadNotificationCounter( echoApi, [ 'message', 'alert' ], limitNotifications ),
-			modelManager = new mw.echo.dm.ModelManager( unreadCounter, { type: [ 'message', 'alert' ] } ),
+			modelManager = new mw.echo.dm.ModelManager( unreadCounter, {
+				type: [ 'message', 'alert' ],
+				itemsPerPage: limitNotifications
+			} ),
 			controller = new mw.echo.Controller(
 				echoApi,
-				modelManager,
-				{
-					type: [ 'message' ]
-				}
+				modelManager
 			),
 			specialPageContainer = new mw.echo.ui.NotificationsInboxWidget(
 				controller,

modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js

@@ -82,7 +82,7 @@
 		this.actionsButtonSelectWidget.addItems( [ this.toggleExpandButton ] );
 
 		// Events
-		this.model.connect( this, { removeSource: 'onModelRemoveSource' } );
+		this.model.connect( this, { discard: 'onModelDiscard' } );
 		this.toggleExpandButton.connect( this, { click: 'expand' } );
 		this.$content.on( 'click', this.expand.bind( this ) );
 
@@ -120,11 +120,11 @@
 	/**
 	 * Respond to model removing source group
 	 *
-	 * @param {string} source Symbolic name of the source group
+	 * @param {string} groupName Symbolic name of the group
 	 */
-	mw.echo.ui.CrossWikiNotificationItemWidget.prototype.onModelRemoveSource = function ( source ) {
+	mw.echo.ui.CrossWikiNotificationItemWidget.prototype.onModelDiscard = function ( groupName ) {
 		var list = this.getList(),
-			group = list.getItemFromId( source );
+			group = list.getItemFromId( groupName );
 
 		list.removeItems( [ group ] );
 

modules/ui/mw.echo.ui.DatedNotificationsWidget.js

@@ -42,7 +42,7 @@
 		// Events
 		this.manager.connect( this, {
 			update: 'populateFromModel',
-			removeSource: 'onModelRemoveSource'
+			discard: 'onManagerDiscardModel'
 		} );
 
 		this.$element
@@ -57,6 +57,16 @@
 	OO.inheritClass( mw.echo.ui.DatedNotificationsWidget, OO.ui.Widget );
 	OO.mixinClass( mw.echo.ui.DatedNotificationsWidget, OO.ui.mixin.PendingElement );
 
+	mw.echo.ui.DatedNotificationsWidget.prototype.onManagerDiscardModel = function ( modelId ) {
+		var group,
+			model = this.models[ modelId ],
+			list = this.getList();
+
+		if ( model ) {
+			group = list.getItemFromId( model.getName() );
+			list.removeItems( [ group ] );
+		}
+	};
 	/**
 	 * Respond to model removing source group
 	 *
@@ -94,6 +104,8 @@
 					$overlay: this.$overlay
 				}
 			);
+			this.attachModel( model, models[ model ] );
+
 			subgroupWidget.resetItemsFromModel();
 			groupWidgets.push( subgroupWidget );
 		}

modules/ui/mw.echo.ui.NotificationsInboxWidget.js

@@ -70,7 +70,9 @@
 		// Events
 		this.readStateSelectWidget.connect( this, { filter: 'onReadStateFilter' } );
 		this.xwikiUnreadWidget.connect( this, { filter: 'onSourcePageFilter' } );
+		this.manager.connect( this, { modelItemUpdate: 'onManagerModelItemUpdate' } );
 		this.manager.getFiltersModel().connect( this, { update: 'updateReadStateSelectWidget' } );
+		this.manager.getPaginationModel().connect( this, { update: 'onPaginationModelUpdate' } );
 		this.topPaginationWidget.connect( this, { change: 'populateNotifications' } );
 		this.bottomPaginationWidget.connect( this, { change: 'populateNotifications' } );
 
@@ -154,6 +156,22 @@
 			.setSelected( true );
 	};
 
+	/**
+	 * Respond to pagination model update event
+	 */
+	mw.echo.ui.NotificationsInboxWidget.prototype.onPaginationModelUpdate = function () {
+		this.resetMessageLabel();
+	};
+
+	/**
+	 * Respond to a change in model item
+	 */
+	mw.echo.ui.NotificationsInboxWidget.prototype.onManagerModelItemUpdate = function () {
+		// Update the pagination label
+		this.topPaginationWidget.updateLabel();
+		this.bottomPaginationWidget.updateLabel();
+	};
+
 	/**
 	 * Respond to unread page filter
 	 *
@@ -239,7 +257,7 @@
 	 */
 	mw.echo.ui.NotificationsInboxWidget.prototype.resetMessageLabel = function () {
 		var label,
-			count = this.manager.getAllNotificationCount();
+			count = this.manager.getPaginationModel().getCurrentPageItemCount();
 
 		if ( count === 0 ) {
 			label = this.manager.getFiltersModel().getReadState() === 'all' ?

modules/ui/mw.echo.ui.NotificationsListWidget.js

@@ -59,7 +59,7 @@
 
 		this.manager.connect( this, {
 			update: 'resetDataFromModel',
-			remove: 'onModelManagerRemove'
+			discard: 'onModelManagerDiscard'
 		} );
 
 		this.$element
@@ -72,7 +72,7 @@
 
 	/* Methods */
 
-	mw.echo.ui.NotificationsListWidget.prototype.onModelManagerRemove = function ( modelName ) {
+	mw.echo.ui.NotificationsListWidget.prototype.onModelManagerDiscard = function ( modelName ) {
 		var i,
 			items = this.getItems();
 

modules/ui/mw.echo.ui.PaginationWidget.js

@@ -152,8 +152,7 @@
 	 */
 	mw.echo.ui.PaginationWidget.prototype.updateLabel = function () {
 		var label,
-			itemsInPage = this.model.hasNextPage() ?
-				this.itemsPerPage : this.model.getLastPageItemCount(),
+			itemsInPage = this.model.getCurrentPageItemCount(),
 			firstNotifNum = this.model.getCurrPageIndex() * this.itemsPerPage,
 			lastNotifNum = firstNotifNum + itemsInPage;