From b082e920b17e9a778e876514ff18070d429bd5f6 Mon Sep 17 00:00:00 2001 From: lupo Date: Mon, 18 Jun 2012 08:43:47 +0200 Subject: [PATCH] (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 | 51 ++++++++++++++++++++++------- includes/Title.php | 52 +++++++++++++++++++++++------- includes/User.php | 32 ++++++++++++------ includes/WikiPage.php | 8 ++--- languages/messages/MessagesEn.php | 4 +++ languages/messages/MessagesQqq.php | 12 +++++++ maintenance/language/messages.inc | 4 +++ 7 files changed, 127 insertions(+), 36 deletions(-) diff --git a/includes/Skin.php b/includes/Skin.php index 8d47b83d0f..fa99772ef1 100644 --- a/includes/Skin.php +++ b/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 ); } diff --git a/includes/Title.php b/includes/Title.php index 481f480c99..c9bafe00ec 100644 --- a/includes/Title.php +++ b/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. - */ - public function countAuthorsBetween( $old, $new, $limit ) { + * @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, $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 ); diff --git a/includes/User.php b/includes/User.php index 01407b1d8e..58de949792 100644 --- a/includes/User.php +++ b/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 ); } diff --git a/includes/WikiPage.php b/includes/WikiPage.php index b5f4c1dfa6..991189dce6 100644 --- a/includes/WikiPage.php +++ b/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. diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index 1fdb65ad33..f49522d19d 100644 --- a/languages/messages/MessagesEn.php +++ b/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', diff --git a/languages/messages/MessagesQqq.php b/languages/messages/MessagesQqq.php index f0ca5e4752..017523e384 100644 --- a/languages/messages/MessagesQqq.php +++ b/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}}]. diff --git a/maintenance/language/messages.inc b/maintenance/language/messages.inc index 493fcc55cf..d047316c33 100644 --- a/maintenance/language/messages.inc +++ b/maintenance/language/messages.inc @@ -306,6 +306,10 @@ $wgMessageStructure = array( 'youhavenewmessages', 'newmessageslink', 'newmessagesdifflink', + 'youhavenewmessagesfromusers', + 'youhavenewmessagesmanyusers', + 'newmessageslinkplural', + 'newmessagesdifflinkplural', 'youhavenewmessagesmulti', 'newtalkseparator', 'editsection', -- 2.20.1