(bug 12701) Use diff of all unseen revisions in the "new messages" bar.

Also pluralize properly.

"You have a new message from another user (last change)"
if only one unseen revision, or
"You have new messages [from another user|from N users|]
(last changes)" if there are several unseen revisions.

Contains a fix in Title::countAuthorsBetween() adding options to include
(one or both of) the delimiting revisions in the count.

Change-Id: I8870111802085d0bd188cb508c4f4b852985634d

includes/Skin.php

@@ -72,7 +72,7 @@ abstract class Skin extends ContextSource {
 		}
 		return $wgValidSkinNames;
 	}
- 
+
  	/**
 	 * Fetch the skinname messages for available skins.
 	 * @return array of strings
@@ -1342,29 +1342,58 @@ abstract class Skin extends ContextSource {
 		$ntl = '';
 
 		if ( count( $newtalks ) == 1 && $newtalks[0]['wiki'] === wfWikiID() ) {
-			$userTitle = $this->getUser()->getUserPage();
-			$userTalkTitle = $userTitle->getTalkPage();
+			$userTalkTitle = $this->getUser()->getTalkPage();
 
 			if ( !$userTalkTitle->equals( $out->getTitle() ) ) {
+				$lastSeenRev = isset( $newtalks[0]['rev'] ) ? $newtalks[0]['rev'] : null;
+				$nofAuthors = 0;
+				if ( $lastSeenRev !== null ) {
+					$plural = true; // Default if we have a last seen revision: if unknown, use plural
+					$latestRev = Revision::newFromTitle ($userTalkTitle);
+					if ( $latestRev !== null ) {
+						// Singular if only 1 unseen revision, plural if several unseen revisions.
+						$plural = $latestRev->getParentId() !== $lastSeenRev->getId();
+						$nofAuthors = $userTalkTitle->countAuthorsBetween( $lastSeenRev, $latestRev, 10, 'include_new' );
+					}
+				} else {
+					// Singular if no revision -> diff link will show latest change only in any case
+					$plural = false;
+				}
+				$plural = $plural ? 2 : 1;
+				// 2 signifies "more than one revision". We don't know how many, and even if we did,
+				// the number of revisions or authors is not necessarily the same as the number of
+				// "messages".
 				$newMessagesLink = Linker::linkKnown(
 					$userTalkTitle,
-					$this->msg( 'newmessageslink' )->escaped(),
+					$this->msg( 'newmessageslinkplural' )->params( $plural )->escaped(),
 					array(),
 					array( 'redirect' => 'no' )
 				);
 
 				$newMessagesDiffLink = Linker::linkKnown(
 					$userTalkTitle,
-					$this->msg( 'newmessagesdifflink' )->escaped(),
+					$this->msg( 'newmessagesdifflinkplural' )->params( $plural )->escaped(),
 					array(),
-					array( 'diff' => 'cur' )
+					$lastSeenRev !== null
+						? array( 'oldid' => $lastSeenRev->getId(), 'diff' => 'cur' )
+						: array( 'diff' => 'cur' )
 				);
 
-				$ntl = $this->msg(
-					'youhavenewmessages',
-					$newMessagesLink,
-					$newMessagesDiffLink
-				)->text();
+				if ( $nofAuthors >= 1 && $nofAuthors <= 10 ) {
+					$ntl = $this->msg(
+						'youhavenewmessagesfromusers',
+						$newMessagesLink,
+						$newMessagesDiffLink
+					)->numParams( $nofAuthors );
+				} else {
+					// $nofAuthors === 11 signifies "11 or more" ("more than 10")
+					$ntl = $this->msg(
+						$nofAuthors > 10 ? 'youhavenewmessagesmanyusers' : 'youhavenewmessages',
+						$newMessagesLink,
+						$newMessagesDiffLink
+					);
+				}
+				$ntl = $ntl->text();
 				# Disable Squid cache
 				$out->setSquidMaxage( 0 );
 			}

includes/Title.php

@@ -825,7 +825,7 @@ class Title {
 
 	/**
 	 * Returns true if the title is inside the specified namespace.
-	 * 
+	 *
 	 * Please make use of this instead of comparing to getNamespace()
 	 * This function is much more resistant to changes we may make
 	 * to namespaces than code that makes direct comparisons.
@@ -1881,7 +1881,7 @@ class Title {
 					$title_protection['pt_create_perm'] = 'protect'; // B/C
 				}
 				if( $title_protection['pt_create_perm'] == '' ||
-					!$user->isAllowed( $title_protection['pt_create_perm'] ) ) 
+					!$user->isAllowed( $title_protection['pt_create_perm'] ) )
 				{
 					$errors[] = array( 'titleprotected', User::whoIs( $title_protection['pt_user'] ), $title_protection['pt_reason'] );
 				}
@@ -4076,30 +4076,60 @@ class Title {
 	}
 
 	/**
-	 * Get the number of authors between the given revision IDs.
+	 * Get the number of authors between the given revisions or revision IDs.
 	 * Used for diffs and other things that really need it.
 	 *
-	 * @param $old int|Revision Old revision or rev ID (first before range)
-	 * @param $new int|Revision New revision or rev ID (first after range)
-	 * @param $limit Int Maximum number of authors
-	 * @return Int Number of revision authors between these revisions.
+	 * @param $old int|Revision Old revision or rev ID (first before range by default)
+	 * @param $new int|Revision New revision or rev ID (first after range by default)
+	 * @param $limit int Maximum number of authors
+	 * @param $options string|array (Optional): Single option, or an array of options:
+	 *     'include_old' Include $old in the range; $new is excluded.
+	 *     'include_new' Include $new in the range; $old is excluded.
+	 *     'include_both' Include both $old and $new in the range.
+	 *     Unknown option values are ignored.
+	 * @return int Number of revision authors in the range; zero if not both revisions exist
 	 */
-	public function countAuthorsBetween( $old, $new, $limit ) {
+	public function countAuthorsBetween( $old, $new, $limit, $options = array() ) {
 		if ( !( $old instanceof Revision ) ) {
 			$old = Revision::newFromTitle( $this, (int)$old );
 		}
 		if ( !( $new instanceof Revision ) ) {
 			$new = Revision::newFromTitle( $this, (int)$new );
 		}
+		// XXX: what if Revision objects are passed in, but they don't refer to this title?
+		// Add $old->getPage() != $new->getPage() || $old->getPage() != $this->getArticleID()
+		// in the sanity check below?
 		if ( !$old || !$new ) {
 			return 0; // nothing to compare
 		}
+		$old_cmp = '>';
+		$new_cmp = '<';
+		$options = (array) $options;
+		if ( in_array( 'include_old', $options ) ) {
+			$old_cmp = '>=';
+		}
+		if ( in_array( 'include_new', $options ) ) {
+			$new_cmp = '<=';
+		}
+		if ( in_array( 'include_both', $options ) ) {
+			$old_cmp = '>=';
+			$new_cmp = '<=';
+		}
+		// No DB query needed if $old and $new are the same or successive revisions:
+		if ( $old->getId() === $new->getId() ) {
+			return ( $old_cmp === '>' && $new_cmp === '<' ) ? 0 : 1;
+		} else if ( $old->getId() === $new->getParentId() ) {
+			if ( $old_cmp === '>' || $new_cmp === '<' ) {
+				return ( $old_cmp === '>' && $new_cmp === '<' ) ? 0 : 1;
+			}
+			return ( $old->getRawUserText() === $new->getRawUserText() ) ? 1 : 2;
+		}
 		$dbr = wfGetDB( DB_SLAVE );
 		$res = $dbr->select( 'revision', 'DISTINCT rev_user_text',
 			array(
 				'rev_page' => $this->getArticleID(),
-				'rev_timestamp > ' . $dbr->addQuotes( $dbr->timestamp( $old->getTimestamp() ) ),
-				'rev_timestamp < ' . $dbr->addQuotes( $dbr->timestamp( $new->getTimestamp() ) )
+				"rev_timestamp $old_cmp " . $dbr->addQuotes( $dbr->timestamp( $old->getTimestamp() ) ),
+				"rev_timestamp $new_cmp " . $dbr->addQuotes( $dbr->timestamp( $new->getTimestamp() ) )
 			), __METHOD__,
 			array( 'LIMIT' => $limit + 1 ) // add one so caller knows it was truncated
 		);

includes/User.php

@@ -1778,14 +1778,20 @@ class User {
 	 */
 	public function getNewMessageLinks() {
 		$talks = array();
-		if( !wfRunHooks( 'UserRetrieveNewTalks', array( &$this, &$talks ) ) )
+		if( !wfRunHooks( 'UserRetrieveNewTalks', array( &$this, &$talks ) ) ) {
 			return $talks;
-
-		if( !$this->getNewtalk() )
+		} elseif( !$this->getNewtalk() ) {
 			return array();
-		$up = $this->getUserPage();
-		$utp = $up->getTalkPage();
-		return array( array( 'wiki' => wfWikiID(), 'link' => $utp->getLocalURL() ) );
+		}
+		$utp = $this->getTalkPage();
+		$dbr = wfGetDB( DB_SLAVE );
+		// Get the "last viewed rev" timestamp from the oldest message notification
+		$timestamp = $dbr->selectField( 'user_newtalk',
+			'MIN(user_last_timestamp)',
+			$this->isAnon() ? array( 'user_ip' => $this->getName() ) : array( 'user_id' => $this->getID() ),
+			__METHOD__ );
+		$rev = $timestamp ? Revision::loadFromTimestamp( $dbr, $utp, $timestamp ) : null;
+		return array( array( 'wiki' => wfWikiID(), 'link' => $utp->getLocalURL(), 'rev' => $rev ) );
 	}
 
 	/**
@@ -1812,12 +1818,17 @@ class User {
 	 * Add or update the new messages flag
 	 * @param $field String 'user_ip' for anonymous users, 'user_id' otherwise
 	 * @param $id String|Int User's IP address for anonymous users, User ID otherwise
+	 * @param $curRev Revision new, as yet unseen revision of the user talk page. Ignored if null.
 	 * @return Bool True if successful, false otherwise
 	 */
-	protected function updateNewtalk( $field, $id ) {
+	protected function updateNewtalk( $field, $id, $curRev = null ) {
+		// Get timestamp of the talk page revision prior to the current one
+		$prevRev = $curRev ? $curRev->getPrevious() : false;
+		$ts = $prevRev ? $prevRev->getTimestamp() : null;
+		// Mark the user as having new messages since this revision
 		$dbw = wfGetDB( DB_MASTER );
 		$dbw->insert( 'user_newtalk',
-			array( $field => $id ),
+			array( $field => $id, 'user_last_timestamp' => $dbw->timestampOrNull( $ts ) ),
 			__METHOD__,
 			'IGNORE' );
 		if ( $dbw->affectedRows() ) {
@@ -1852,8 +1863,9 @@ class User {
 	/**
 	 * Update the 'You have new messages!' status.
 	 * @param $val Bool Whether the user has new messages
+	 * @param $curRev Revision new, as yet unseen revision of the user talk page. Ignored if null or !$val.
 	 */
-	public function setNewtalk( $val ) {
+	public function setNewtalk( $val, $curRev = null ) {
 		if( wfReadOnly() ) {
 			return;
 		}
@@ -1871,7 +1883,7 @@ class User {
 		global $wgMemc;
 
 		if( $val ) {
-			$changed = $this->updateNewtalk( $field, $id );
+			$changed = $this->updateNewtalk( $field, $id, $curRev );
 		} else {
 			$changed = $this->deleteNewtalk( $field, $id );
 		}

includes/WikiPage.php

@@ -611,7 +611,7 @@ class WikiPage extends Page {
 		if ( !$this->mTimestamp ) {
 			$this->loadLastEdit();
 		}
-		
+
 		return wfTimestamp( TS_MW, $this->mTimestamp );
 	}
 
@@ -1819,9 +1819,9 @@ class WikiPage extends Page {
 					wfDebug( __METHOD__ . ": invalid username\n" );
 				} elseif ( User::isIP( $shortTitle ) ) {
 					// An anonymous user
-					$other->setNewtalk( true );
+					$other->setNewtalk( true, $revision );
 				} elseif ( $other->isLoggedIn() ) {
-					$other->setNewtalk( true );
+					$other->setNewtalk( true, $revision );
 				} else {
 					wfDebug( __METHOD__ . ": don't need to notify a nonexistent user\n" );
 				}
@@ -2280,7 +2280,7 @@ class WikiPage extends Page {
 	 * roll back to, e.g. user is the sole contributor. This function
 	 * performs permissions checks on $user, then calls commitRollback()
 	 * to do the dirty work
-	 * 
+	 *
 	 * @todo: seperate the business/permission stuff out from backend code
 	 *
 	 * @param $fromP String: Name of the user whose edits to rollback.

languages/messages/MessagesEn.php

@@ -906,6 +906,10 @@ See [[Special:Version|version page]].',
 'youhavenewmessages'           => 'You have $1 ($2).',
 'newmessageslink'              => 'new messages',
 'newmessagesdifflink'          => 'last change',
+'youhavenewmessagesfromusers'  => 'You have $1 from {{PLURAL:$3|another user|$3 users}} ($2).',
+'youhavenewmessagesmanyusers'  => 'You have $1 from many users ($2).',
+'newmessageslinkplural'        => '{{PLURAL:$1|a new message|new messages}}', # don't rely on the value of $1, it's 1 for singular and 2 for "more than one"
+'newmessagesdifflinkplural'    => 'last {{PLURAL:$1|change|changes}}', # don't rely on the value of $1, it's 1 for singular and 2 for "more than one"
 'youhavenewmessagesmulti'      => 'You have new messages on $1',
 'newtalkseparator'             => ', ', # do not translate or duplicate this message to other languages
 'editsection'                  => 'edit',

languages/messages/MessagesQqq.php

@@ -546,6 +546,18 @@ The format is: "{{int:youhavenewmessages| [[MediaWiki:Newmessageslink/{{SUBPAGEN
 
 {{Identical|New messages}}',
 'newmessagesdifflink' => 'This is the second link displayed in an orange rectangle when a user gets a message on his talk page. Used in message {{msg-mw|youhavenewmessages}} (as parameter $2).',
+'youhavenewmessagesfromusers' => 'New talk inidcator message: the message appearing when someone edited your user talk page.
+The message takes three parameters, {{msg-mw|newmessageslinkplural}}, {{msg-mw|newmessagesdifflinkplurl}}, and the number of authors
+that have edited the talk page since the owning user last viewed it.',
+'youhavenewmessagesmanyusers' => 'New talk inidcator message: the message appearing when someone edited your user talk page.
+Like {{msg-mw|youhavenewmessages}}, but getting {{msg-mw|newmessageslinkplural}} and {{msg-mw|newmessagesdifflinkplurl}} as parameters $1 and $2, respectively. Used when more than 10 users
+edited the user talk page since the owning user last viewed it.',
+'newmessageslinkplural' => 'Like {{msg-mw|newmessageslink}} but supporting pluralization. Used in message {{msg-mw|youhavenewmessagesfromusers}} (as parameter $1).
+This message itself takes one parameter, $1, which is 1 if there was one new edit, or 2 if there was more than one new edit
+since the last time the user has seen his or her talk page.',
+'newmessagesdifflinkplural' => 'Like {{msg-mw|newmessagesdifflink}} but supporting pluralization. Used in message {{msg-mw|youhavenewmessagesfromusers}} (as parameter $2).
+This message itself takes one parameter, $1, which is 1 if there was one new edit, or 2 if there was more than one new edit
+since the last time the user has seen his or her talk page.',
 'youhavenewmessagesmulti' => 'The alternative of {{msg|youhavenewmessages}} as used on wikis with a special setup so they can receive the "new message" notice on other wikis as well. Used on [http://www.wikia.com/ Wikia].
 The format is: "{{int:youhavenewmessagesmulti| [[MediaWiki:Newmessageslink/{{SUBPAGENAME}}|{{int:newmessageslink}}]]}}"',
 'editsection' => 'Display name of link to edit a section on a content page. Example: [{{MediaWiki:Editsection}}].

maintenance/language/messages.inc

@@ -306,6 +306,10 @@ $wgMessageStructure = array(
 		'youhavenewmessages',
 		'newmessageslink',
 		'newmessagesdifflink',
+		'youhavenewmessagesfromusers',
+		'youhavenewmessagesmanyusers',
+		'newmessageslinkplural',
+		'newmessagesdifflinkplural',
 		'youhavenewmessagesmulti',
 		'newtalkseparator',
 		'editsection',