r38233 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r38232‎ \| r38233 \| r38234 >
Date:	12:30, 30 July 2008
Author:	aaron
Status:	old
Tags:
Comment:	Refactor some code to look better
Modified paths:	/trunk/extensions/FlaggedRevs/FlaggedArticle.php (modified) (history) /trunk/extensions/FlaggedRevs/FlaggedRevision.php (modified) (history) /trunk/extensions/FlaggedRevs/FlaggedRevs.class.php (modified) (history) /trunk/extensions/FlaggedRevs/FlaggedRevs.php (modified) (history) /trunk/extensions/FlaggedRevs/FlaggedRevsPage.php (modified) (history)

Diff [purge]

Index: trunk/extensions/FlaggedRevs/FlaggedRevs.php
—	—	@@ -37,6 +37,11 @@
38	38	# Pristine -> Quality -> Sighted
39	39	if( !defined('FLAGGED_VIS_PRISTINE') )
40	40	define('FLAGGED_VIS_PRISTINE',2);
	41	+
	42	+if( !defined('FR_FOR_UPDATE') )
	43	+ define('FR_FOR_UPDATE',1);
	44	+if( !defined('FR_TEXT') )
	45	+ define('FR_TEXT',2);
41	46
42	47	$wgExtensionCredits['specialpage'][] = array(
43	48	'name' => 'Flagged Revisions',
Index: trunk/extensions/FlaggedRevs/FlaggedArticle.php
—	—	@@ -207,7 +207,7 @@
208	208	$simpleTag = $old = $stable = false;
209	209	$tag = $prot = $notes = $pending = '';
210	210	# Check the newest stable version.
211		~~- $srev = $this->getStableRev( true );~~
	211	+ $srev = $this->getStableRev( FR_TEXT );
212	212	$frev = $srev;
213	213	$stableId = $frev ? $frev->getRevId() : 0;
214	214	# Also, check for any explicitly requested old stable version...
—	—	@@ -217,7 +217,7 @@
218	218	}
219	219	if( $stableId && $reqId ) {
220	220	if( $reqId != $stableId ) {
221		~~- $frev = FlaggedRevision::newFromTitle( $this->parent->getTitle(), $reqId, true );~~
	221	+ $frev = FlaggedRevision::newFromTitle( $this->parent->getTitle(), $reqId, FR_TEXT );
222	222	$old = true; // old reviewed version requested by ID
223	223	if( !$frev ) {
224	224	$wgOut->addWikiText( wfMsg('revreview-invalid') );
—	—	@@ -460,7 +460,7 @@
461	461	if( $reqId = $wgRequest->getVal('stableid') ) {
462	462	$frev = FlaggedRevision::newFromTitle( $this->parent->getTitle(), $reqId );
463	463	} else if( $this->pageOverride() ) {
464		~~- $frev = $this->getStableRev( true );~~
	464	+ $frev = $this->getStableRev( FR_TEXT );
465	465	}
466	466	if( !is_null($frev) ) {
467	467	$time = $frev->getFileTimestamp();
—	—	@@ -717,7 +717,7 @@
718	718	$action = $wgRequest->getVal( 'action', 'view' );
719	719	if( $action == 'protect' \|\| $action == 'unprotect' ) {
720	720	# Check for an overridabe revision
721		~~- $frev = $this->getStableRev( true );~~
	721	+ $frev = $this->getStableRev( FR_TEXT );
722	722	if( !$frev )
723	723	return true;
724	724	# Load special page name
—	—	@@ -757,7 +757,7 @@
758	758	}
759	759	# If we are viewing a page normally, and it was overridden,
760	760	# change the edit tab to a "current revision" tab
761		~~- $srev = $this->getStableRev( true );~~
	761	+ $srev = $this->getStableRev( FR_TEXT );
762	762	# No quality revs? Find the last reviewed one
763	763	if( is_null($srev) ) {
764	764	return true;
—	—	@@ -1154,7 +1154,7 @@
1155	1155	if( !$this->isReviewable() \|\| $this->parent->getTitle()->isTalkPage() )
1156	1156	return true;
1157	1157	# Get the stable version, from master
1158		~~- $frev = $this->getStableRev( false, true );~~
	1158	+ $frev = $this->getStableRev( FR_FOR_UPDATE );
1159	1159	if( !$frev )
1160	1160	return true;
1161	1161	$latest = $this->parent->getTitle()->getLatestRevID(GAID_FOR_UPDATE);
—	—	@@ -1201,11 +1201,10 @@
1202	1202
1203	1203	/**
1204	1204	* Get latest quality rev, if not, the latest reviewed one
1205		~~- * @param Bool $getText, get text and params columns?~~
1206		~~- * @param Bool $forUpdate, use DB master and avoid page table?~~
	1205	+ * @param int $flags
1207	1206	* @return Row
1208	1207	*/
1209		~~- public function getStableRev( $getText = false, $forUpdate = false ) {~~
	1208	+ public function getStableRev( $flags=0 ) {
1210	1209	if( $this->stableRev === false ) {
1211	1210	return null; // We already looked and found nothing...
1212	1211	}
—	—	@@ -1216,7 +1215,7 @@
1217	1216	# Get the content page, skip talk
1218	1217	$title = $this->parent->getTitle()->getSubjectPage();
1219	1218	# Do we have one?
1220		~~- $srev = FlaggedRevision::newFromStable( $title, $getText, $forUpdate );~~
	1219	+ $srev = FlaggedRevision::newFromStable( $title, $flags );
1221	1220	if( $srev ) {
1222	1221	$this->stableRev = $srev;
1223	1222	$this->flags[$srev->getRevId()] = $srev->getTags();
—	—	@@ -1287,7 +1286,7 @@
1288	1287	# If we are reviewing updates to a page, start off with the stable revision's
1289	1288	# flags. Otherwise, we just fill them in with the selected revision's flags.
1290	1289	if( $this->isDiffFromStable ) {
1291		~~- $srev = $this->getStableRev( true );~~
	1290	+ $srev = $this->getStableRev( FR_TEXT );
1292	1291	$flags = $srev->getTags();
1293	1292	# Check if user is allowed to renew the stable version.
1294	1293	# If not, then get the flags for the new revision itself.
Index: trunk/extensions/FlaggedRevs/FlaggedRevs.class.php
—	—	@@ -985,7 +985,7 @@
986	986	$title = $article->getTitle();
987	987
988	988	# Get current stable version ID (for logging)
989		~~- $oldSv = FlaggedRevision::newFromStable( $title, false, true );~~
	989	+ $oldSv = FlaggedRevision::newFromStable( $title, FR_TEXT \| FR_FOR_UPDATE );
990	990	$oldSvId = $oldSv ? $oldSv->getRevId() : 0;
991	991
992	992	# Rev ID is not put into parser on edit, so do the same here.
—	—	@@ -1108,7 +1108,7 @@
1109	1109
1110	1110	# If we know that this is now the new stable version
1111	1111	# (which it probably is), save it to the cache...
1112		~~- $sv = FlaggedRevision::newFromStable( $article->getTitle(), false, true );~~
	1112	+ $sv = FlaggedRevision::newFromStable( $article->getTitle(), FR_FOR_UPDATE );
1113	1113	if( $sv && $sv->getRevId() == $rev->getId() ) {
1114	1114	# Update stable cache
1115	1115	self::updatePageCache( $article, $poutput );
—	—	@@ -1156,7 +1156,7 @@
1157	1157	$stylePath = str_replace( '$wgScriptPath', $wgScriptPath, $wgFlaggedRevsStylePath );
1158	1158	$rTags = self::getJSTagParams();
1159	1159	$fTags = self::getJSFeedbackParams();
1160		~~- $frev = $flaggedArticle->getStableRev( true );~~
	1160	+ $frev = $flaggedArticle->getStableRev( FR_TEXT );
1161	1161	$stableId = $frev ? $frev->getRevId() : 0;
1162	1162	$encCssFile = htmlspecialchars( "$stylePath/flaggedrevs.css?$wgFlaggedRevStyleVersion" );
1163	1163	$encJsFile = htmlspecialchars( "$stylePath/flaggedrevs.js?$wgFlaggedRevStyleVersion" );
—	—	@@ -1294,7 +1294,8 @@
1295	1295	# Check if this page has a stable version by fetching it. Do not
1296	1296	# get the fr_text field if we are to use the latest stable template revisions.
1297	1297	global $wgUseStableTemplates;
1298		~~- $sv = FlaggedRevision::newFromStable( $linksUpdate->mTitle, !$wgUseStableTemplates, true );~~
	1298	+ $flags = $wgUseStableTemplates ? FR_FOR_UPDATE : FR_FOR_UPDATE \| FR_TEXT;
	1299	+ $sv = FlaggedRevision::newFromStable( $linksUpdate->mTitle, $flags );
1299	1300	if( !$sv ) {
1300	1301	$dbw = wfGetDB( DB_MASTER );
1301	1302	$dbw->delete( 'flaggedpages',
—	—	@@ -1445,7 +1446,7 @@
1446	1447	$sha1 = "";
1447	1448	global $wgUseStableImages;
1448	1449	if( $wgUseStableImages && self::isPageReviewable( $title ) ) {
1449		~~- $srev = FlaggedRevision::newFromStable( $title, false, true );~~
	1450	+ $srev = FlaggedRevision::newFromStable( $title, FR_FOR_UPDATE );
1450	1451	if( $srev ) {
1451	1452	$time = $srev->getFileTimestamp();
1452	1453	$sha1 = $srev->getFileSha1();
—	—	@@ -1510,7 +1511,7 @@
1511	1512	$sha1 = "";
1512	1513	global $wgUseStableImages;
1513	1514	if( $wgUseStableImages && self::isPageReviewable( $nt ) ) {
1514		~~- $srev = FlaggedRevision::newFromStable( $nt, false, true );~~
	1515	+ $srev = FlaggedRevision::newFromStable( $nt, FR_FOR_UPDATE );
1515	1516	if( $srev ) {
1516	1517	$time = $srev->getFileTimestamp();
1517	1518	$sha1 = $srev->getFileSha1();
—	—	@@ -1664,7 +1665,7 @@
1665	1666	$flaggedArticle = FlaggedArticle::getTitleInstance( $title );
1666	1667	if( $wgTitle && $wgTitle->equals( $title ) ) {
1667	1668	// Cache stable version while we are at it.
1668		~~- if( $flaggedArticle->pageOverride() && $flaggedArticle->getStableRev( true ) ) {~~
	1669	+ if( $flaggedArticle->pageOverride() && $flaggedArticle->getStableRev( FR_TEXT ) ) {
1669	1670	$result = true;
1670	1671	}
1671	1672	} else {
—	—	@@ -1703,11 +1704,12 @@
1704	1705	$reviewableNewPage = ( $wgFlaggedRevsAutoReviewNew && $user->isAllowed('review') );
1705	1706	// Edits to existing pages
1706	1707	} else if( $baseRevID ) {
1707		~~- $frev = FlaggedRevision::newFromTitle( $title, $baseRevID, false, true, $rev->getPage() );~~
	1708	+ $title->resetArticleID( $rev->getPage() ); // avoid db hit
	1709	+ $frev = FlaggedRevision::newFromTitle( $title, $baseRevID, FR_FOR_UPDATE );
1708	1710	# If the base revision was not reviewed, check if the previous one was.
1709	1711	# This should catch null edits as well as normal ones.
1710	1712	if( !$frev ) {
1711		~~- $frev = FlaggedRevision::newFromTitle( $title, $prevRevID, false, true, $rev->getPage() );~~
	1713	+ $frev = FlaggedRevision::newFromTitle( $title, $prevRevID, FR_FOR_UPDATE );
1712	1714	}
1713	1715	}
1714	1716	# Is this an edit directly to the stable version?
—	—	@@ -2103,7 +2105,7 @@
2104	2106	} else {
2105	2107	# Get an instance on the title ($wgTitle) and save to process cache
2106	2108	$flaggedArticle = FlaggedArticle::getTitleInstance( $title );
2107		~~- if( $flaggedArticle->pageOverride() && $srev = $flaggedArticle->getStableRev( true ) ) {~~
	2109	+ if( $flaggedArticle->pageOverride() && $srev = $flaggedArticle->getStableRev( FR_TEXT ) ) {
2108	2110	$text = $srev->getRevText();
2109	2111	$redirect = $flaggedArticle->followRedirectText( $text );
2110	2112	if( $redirect ) {
—	—	@@ -2228,12 +2230,12 @@
2229	2231	$fa = FlaggedArticle::getInstance( $article );
2230	2232	# If the stable is the default, and we are viewing it...cache it!
2231	2233	if( $fa->showStableByDefault() ) {
2232		~~- return ( $fa->pageOverride() && $fa->getStableRev( true ) );~~
	2234	+ return ( $fa->pageOverride() && $fa->getStableRev( FR_TEXT ) );
2233	2235	# If the draft is the default, and we are viewing it...cache it!
2234	2236	} else {
2235	2237	global $wgRequest;
2236	2238	# We don't want to cache the pending edit notice though
2237		~~- return !( $fa->pageOverride() && $fa->getStableRev( true ) ) && !$wgRequest->getVal('shownotice');~~
	2239	+ return !( $fa->pageOverride() && $fa->getStableRev( FR_TEXT ) ) && !$wgRequest->getVal('shownotice');
2238	2240	}
2239	2241	}
2240	2242
Index: trunk/extensions/FlaggedRevs/FlaggedRevision.php
—	—	@@ -45,20 +45,17 @@
46	46	/**
47	47	* @param Title $title
48	48	* @param int $revId
49		~~- * @param bool $getText, fetch fr_text and fr_flags too?~~
50		~~- * @param bool $forUpdate, use master?~~
51		~~- * @param int $pageId, optional page ID to use, will defer to $title if not given~~
	49	+ * @param int $flags
52	50	* @returns mixed FlaggedRevision (null on failure)
53	51	* Will not return a revision if deleted
54	52	*/
55		~~- public static function newFromTitle( $title, $revId, $getText=false, $forUpdate=false, $pageId=false ) {~~
	53	+ public static function newFromTitle( $title, $revId, $flags = 0 ) {
56	54	$columns = self::selectFields();
57		~~- if( $getText ) {~~
	55	+ if( $flags & FR_TEXT ) {
58	56	$columns += self::selectTextFields();
59	57	}
60		~~- $db = $forUpdate ? wfGetDB( DB_MASTER ) : wfGetDB( DB_SLAVE );~~
61		~~- $flags = $forUpdate ? GAID_FOR_UPDATE : 0;~~
62		~~- $pageId = $pageId ? $pageId : $title->getArticleID( $flags );~~
	58	+ $db = $flags & FR_FOR_UPDATE ? wfGetDB( DB_MASTER ) : wfGetDB( DB_SLAVE );
	59	+ $pageId = $title->getArticleID( $flags & FR_FOR_UPDATE ? GAID_FOR_UPDATE : 0 );
63	60	# Short-circuit query
64	61	if( !$pageId ) {
65	62	return null;
—	—	@@ -82,13 +79,12 @@
83	80	/**
84	81	* Get latest quality rev, if not, the latest reviewed one.
85	82	* @param Title $title, page title
86		~~- * @param bool $getText, fetch fr_text and fr_flags too?~~
87		~~- * @param bool $forUpdate, use master DB and avoid using fp_stable?~~
	83	+ * @param int $flags
88	84	* @returns mixed FlaggedRevision (null on failure)
89	85	*/
90		~~- public static function newFromStable( $title, $getText=false, $forUpdate=false ) {~~
	86	+ public static function newFromStable( $title, $flags=0 ) {
91	87	$columns = self::selectFields();
92		~~- if( $getText ) {~~
	88	+ if( $flags & FR_TEXT ) {
93	89	$columns += self::selectTextFields();
94	90	}
95	91	$row = null;
—	—	@@ -97,7 +93,7 @@
98	94	return $row;
99	95	}
100	96	# If we want the text, then get the text flags too
101		~~- if( !$forUpdate ) {~~
	97	+ if( !($flags & FR_FOR_UPDATE) ) {
102	98	$dbr = wfGetDB( DB_SLAVE );
103	99	$row = $dbr->selectRow( array('flaggedpages','flaggedrevs'),
104	100	$columns,
Index: trunk/extensions/FlaggedRevs/FlaggedRevsPage.php
—	—	@@ -499,12 +499,12 @@
500	500	}
501	501
502	502	# Get current stable version ID (for logging)
503		~~- $oldSv = FlaggedRevision::newFromStable( $this->page, false, true );~~
	503	+ $oldSv = FlaggedRevision::newFromStable( $this->page, FR_FOR_UPDATE );
504	504	$oldSvId = $oldSv ? $oldSv->getRevId() : 0;
505	505
506	506	# Is this rev already flagged?
507	507	$flaggedOutput = false;
508		~~- if( $oldfrev = FlaggedRevision::newFromTitle( $this->page, $rev->getId(), true, true ) ) {~~
	508	+ if( $oldfrev = FlaggedRevision::newFromTitle( $this->page, $rev->getId(), FR_TEXT \| FR_FOR_UPDATE ) ) {
509	509	$flaggedOutput = FlaggedRevs::parseStableText( $article, $oldfrev->getTextForParse(), $oldfrev->getRevId() );
510	510	}
511	511
—	—	@@ -638,7 +638,7 @@
639	639	}
640	640	# If we know that this is now the new stable version
641	641	# (which it probably is), save it to the stable cache...
642		~~- $sv = FlaggedRevision::newFromStable( $this->page, false, true );~~
	642	+ $sv = FlaggedRevision::newFromStable( $this->page, FR_FOR_UPDATE );
643	643	if( $sv && $sv->getRevId() == $rev->getId() ) {
644	644	# Clear the cache...
645	645	$this->page->invalidateCache();
—	—	@@ -685,7 +685,7 @@
686	686	$dbw->commit();
687	687
688	688	# Get current stable version ID (for logging)
689		~~- $oldSv = FlaggedRevision::newFromStable( $this->page, false, true );~~
	689	+ $oldSv = FlaggedRevision::newFromStable( $this->page, FR_FOR_UPDATE );
690	690	$oldSvId = $oldSv ? $oldSv->getRevId() : 0;
691	691
692	692	# Update the article review log

Status & tagging log

15:29, 12 September 2011 Meno25 (talk | contribs) changed the status of r38233 [removed: ok added: old]