From fc5bcf1147f91fccb946a7126461a33d8cc39762 Mon Sep 17 00:00:00 2001 From: Marius Hoch Date: Sun, 9 Jun 2013 03:41:48 +0200 Subject: [PATCH] Remove revision patrol links from normal page views Users want and expect it this way, per bug 49123 Follow up to I1e24733c Bug: 49123 Change-Id: Ib4d72179e4029f0c089c3147bdf4bd6daac0374e --- includes/Article.php | 106 ++++++++------------------ includes/ChangesList.php | 8 -- includes/diff/DifferenceEngine.php | 8 +- includes/specials/SpecialNewpages.php | 7 -- 4 files changed, 34 insertions(+), 95 deletions(-) diff --git a/includes/Article.php b/includes/Article.php index f0eb568ada..c8e6ce225e 100644 --- a/includes/Article.php +++ b/includes/Article.php @@ -1054,87 +1054,49 @@ class Article implements Page { * @return bool */ public function showPatrolFooter() { - global $wgUseRCPatrol, $wgUseNPPatrol, $wgRCMaxAge, $wgEnableAPI, $wgEnableWriteAPI; + global $wgUseNPPatrol, $wgEnableAPI, $wgEnableWriteAPI; - $request = $this->getContext()->getRequest(); $outputPage = $this->getContext()->getOutput(); $user = $this->getContext()->getUser(); $cache = wfGetMainCache(); + $rc = false; - // Normally this makes a link to patrol the revision being viewed. - // Sometimes, the 'patrolpage' flag is set in case we want to patrol the first - // revision and not the one being viewed by the user (set in Special:NewPages). - $useRCPatrol = $wgUseRCPatrol && !$request->getBool( 'patrolpage' ); - - if ( !$this->getTitle()->quickUserCan( 'patrol', $user ) || ( !$wgUseNPPatrol && !$wgUseRCPatrol ) ) { - // Patrolling is fully disabled or the user isn't allowed to + if ( !$this->getTitle()->quickUserCan( 'patrol', $user ) || !$wgUseNPPatrol ) { + // Patrolling is disabled or the user isn't allowed to return false; } wfProfileIn( __METHOD__ ); - if ( $useRCPatrol ) { - // Check for cached results - if ( $cache->get( wfMemcKey( 'NotPatrollableRevId', $this->getRevIdFetched() ) ) ) { - wfProfileOut( __METHOD__ ); - return false; - } - - // We make use of a little index trick over here: - // First we get the timestamp of the last revision and then - // we look up the RC row by that as the timestamp is indexed - // and usually very few rows exist for one timestamp - // (While several thousand can exists for a single page) - if ( !$this->mRevision ) { - $this->mRevision = Revision::newFromId( $this->getRevIdFetched() ); - } + // New page patrol: Get the timestamp of the oldest revison which + // the revision table holds for the given page. Then we look + // whether it's within the RC lifespan and if it is, we try + // to get the recentchanges row belonging to that entry + // (with rc_new = 1). - if ( !$this->mRevision || !RecentChange::isInRCLifespan( $this->mRevision->getTimestamp(), 21600 ) ) { - // The revision is more than 6 hours older than the Max RC age - // no need to torture the DB any further (6h because the RC might not be cleaned out regularly) - wfProfileOut( __METHOD__ ); - return false; - } - $rc = RecentChange::newFromConds( - array( - 'rc_this_oldid' => $this->getRevIdFetched(), - 'rc_timestamp' => $this->mRevision->getTimestamp(), - 'rc_cur_id' => $this->getTitle()->getArticleID(), - 'rc_patrolled' => 0 - ), - __METHOD__, - array( 'USE INDEX' => 'rc_timestamp' ) - ); - } else { - // RC patrol is disabled so we have to patrol the first - // revision (new page patrol) in case it's in the RC table. - // To achieve this we get the timestamp of the oldest revison - // the revision table holds for the given page. Then we look - // whether it's within the RC lifespan and if it is, we try - // to get the recentchanges row belonging to that entry - // (with rc_new = 1). - - // Check for cached results - if ( $cache->get( wfMemcKey( 'NotPatrollablePage', $this->getTitle()->getArticleID() ) ) ) { - wfProfileOut( __METHOD__ ); - return false; - } + // Check for cached results + if ( $cache->get( wfMemcKey( 'NotPatrollablePage', $this->getTitle()->getArticleID() ) ) ) { + wfProfileOut( __METHOD__ ); + return false; + } - $dbr = wfGetDB( DB_SLAVE ); - $oldestRevisionTimestamp = $dbr->selectField( - 'revision', - 'MIN( rev_timestamp )', - array( 'rev_page' => $this->getTitle()->getArticleID() ), - __METHOD__ - ); + if ( $this->mRevision && !RecentChange::isInRCLifespan( $this->mRevision->getTimestamp(), 21600 ) ) { + // The current revision is already older than what could be in the RC table + // 6h tolerance because the RC might not be cleaned out regularly + wfProfileOut( __METHOD__ ); + return false; + } - if ( !$oldestRevisionTimestamp || !RecentChange::isInRCLifespan( $oldestRevisionTimestamp, 21600 ) ) { - // We either didn't find the oldest revision for the given page - // or it's to old for the RC table (with 6h tolerance) - wfProfileOut( __METHOD__ ); - return false; - } + $dbr = wfGetDB( DB_SLAVE ); + $oldestRevisionTimestamp = $dbr->selectField( + 'revision', + 'MIN( rev_timestamp )', + array( 'rev_page' => $this->getTitle()->getArticleID() ), + __METHOD__ + ); + if ( $oldestRevisionTimestamp && RecentChange::isInRCLifespan( $oldestRevisionTimestamp, 21600 ) ) { + // 6h tolerance because the RC might not be cleaned out regularly $rc = RecentChange::newFromConds( array( 'rc_new' => 1, @@ -1148,19 +1110,14 @@ class Article implements Page { ); } - wfProfileOut( __METHOD__ ); - if ( !$rc ) { // No RC entry around // Cache the information we gathered above in case we can't patrol // Don't cache in case we can patrol as this could change - if( $useRCPatrol ) { - $cache->set( wfMemcKey( 'NotPatrollableRevId', $this->getRevIdFetched() ), '1', $wgRCMaxAge ); - } else { - $cache->set( wfMemcKey( 'NotPatrollablePage', $this->getTitle()->getArticleID() ), '1', $wgRCMaxAge ); - } + $cache->set( wfMemcKey( 'NotPatrollablePage', $this->getTitle()->getArticleID() ), '1' ); + wfProfileOut( __METHOD__ ); return false; } @@ -1190,6 +1147,7 @@ class Article implements Page { '' ); + wfProfileOut( __METHOD__ ); return true; } diff --git a/includes/ChangesList.php b/includes/ChangesList.php index e0f522ae72..02c02e0d28 100644 --- a/includes/ChangesList.php +++ b/includes/ChangesList.php @@ -366,14 +366,6 @@ class ChangesList extends ContextSource { $params = array(); - // In case we got a page creation which is yet unpatrolled and - // recent changes patrolling is enabled, the user probably rather - // wants to patrol the whole page (first revision) instead of seeing - // a patrollink for the current revision. - if ( $wgUseRCPatrol && $unpatrolled && $rc->getAttribute( 'rc_type' ) == RC_NEW ) { - $params['patrolpage'] = 1; - } - $articlelink = Linker::linkKnown( $rc->getTitle(), null, diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php index 4ee066e9e3..916ab38c13 100644 --- a/includes/diff/DifferenceEngine.php +++ b/includes/diff/DifferenceEngine.php @@ -410,8 +410,7 @@ class DifferenceEngine extends ContextSource { * @return String */ protected function markPatrolledLink() { - global $wgUseRCPatrol, $wgRCMaxAge, $wgEnableAPI, $wgEnableWriteAPI; - $cache = wfGetMainCache(); + global $wgUseRCPatrol, $wgEnableAPI, $wgEnableWriteAPI; if ( $this->mMarkPatrolledLink === null ) { // Prepare a change patrol link, if applicable @@ -420,9 +419,7 @@ class DifferenceEngine extends ContextSource { $wgUseRCPatrol && $this->mNewPage->quickUserCan( 'patrol', $this->getUser() ) && // Only do this if the revision isn't more than 6 hours older // than the Max RC age (6h because the RC might not be cleaned out regularly) - RecentChange::isInRCLifespan( $this->mNewRev->getTimestamp(), 21600 ) && - // Maybe the result is cached - !$cache->get( wfMemcKey( 'NotPatrollableRevId', $this->mNewid ) ) + RecentChange::isInRCLifespan( $this->mNewRev->getTimestamp(), 21600 ) ) { // Look for an unpatrolled change corresponding to this diff @@ -464,7 +461,6 @@ class DifferenceEngine extends ContextSource { ) ) . ']'; } else { - $cache->set( wfMemcKey( 'NotPatrollableRevId', $this->mNewid ), '1', $wgRCMaxAge ); $this->mMarkPatrolledLink = ''; } } else { diff --git a/includes/specials/SpecialNewpages.php b/includes/specials/SpecialNewpages.php index ae04061f92..43d485582f 100644 --- a/includes/specials/SpecialNewpages.php +++ b/includes/specials/SpecialNewpages.php @@ -334,13 +334,6 @@ class SpecialNewpages extends IncludableSpecialPage { $query = array( 'redirect' => 'no' ); - if( $this->patrollable( $result ) ) { - // Tell Article.php that we want to patrol the first revision - // and not the current one. Has effect if both recentchages and new page - // patrolling are enabled, we set it everytime for link consistency though. - $query['patrolpage'] = 1; - } - // Linker::linkKnown() uses 'known' and 'noclasses' options. // This breaks the colouration for stubs. $plink = Linker::link( -- 2.20.1