r104444 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r104443‎ | r104444 | r104445 >
Date:15:37, 28 November 2011
Author:cervidae
Status:ok (Comments)
Tags:
Comment:
Merging with Reedy's several code changes. Thanks
Modified paths:
  • /trunk/extensions/HelpCommons/HelpCommons.php (modified) (history)

Diff [purge]

Index: trunk/extensions/HelpCommons/HelpCommons.php
——@@ -30,9 +30,7 @@
3131 $wgExtensionMessagesFiles['HelpCommons'] = $dir . 'HelpCommons.i18n.php';
3232
3333 // Help wiki(s) where the help namespace is fetched from
34 -if( !isset($wgHelpCommonsFetchingWikis) ) {
35 - $wgHelpCommonsFetchingWikis = array();
36 -}
 34+$wgHelpCommonsFetchingWikis = array();
3735
3836 // Hooks
3937 $wgHooks['ShowMissingArticle'][] = 'wfHelpCommonsLoad';
——@@ -41,87 +39,106 @@
4240 $wgHooks['DoEditSectionLink'][] = 'wfHelpCommonsChangeEditSectionLink';
4341 $wgHooks['getUserPermissionsErrors'][] = 'fnProtectHelpCommons';
4442
45 -
 43+/**
 44+ * @param $article Article
 45+ * @return bool
 46+ */
4647 function wfHelpCommonsLoad( $article ) {
4748 global $wgTitle, $wgOut, $wgContLang, $wgHelpCommonsFetchingWikis, $wgLanguageCode, $wgDBname;
4849
49 - if ( $wgTitle->getNamespace() == NS_HELP ) {
 50+ if ( $wgTitle->getNamespace() != NS_HELP ) {
 51+ return false;
 52+ }
5053
51 - $replacewhitespace = str_replace( ' ', '_', $wgOut->getTitle() );
52 - $title = str_replace( $wgContLang->namespaceNames[NS_HELP].':', '', $replacewhitespace );
 54+ $replacewhitespace = str_replace( ' ', '_', $wgOut->getTitle() );
 55+ $title = str_replace( $wgContLang->namespaceNames[NS_HELP].':', '', $replacewhitespace );
5356
54 - foreach ( $wgHelpCommonsFetchingWikis as $language => $urls ) {
55 - foreach ( $urls as $url => $wgHelpCommonsFetchingWiki ) {
56 - if ( $wgLanguageCode == "$language" && $wgDBname != $wgHelpCommonsFetchingWiki ) {
57 - $dbr = wfGetDB( DB_SLAVE, array(), $wgHelpCommonsFetchingWiki );
58 - $page = $dbr->query( 'SELECT page_title, page_namespace, page_latest FROM page WHERE page_namespace = 12 AND page_title = '.$dbr->addQuotes($title) );
59 - $page = $dbr->fetchObject( $page );
60 - }
 57+ foreach ( $wgHelpCommonsFetchingWikis as $language => $urls ) {
 58+ foreach ( $urls as $helpwiki ) {
 59+ if ( $wgLanguageCode == "$language" && $wgDBname != $helpwiki ) {
 60+ $dbr = wfGetDB( DB_SLAVE, array(), $helpwiki );
 61+ $page = $dbr->query( 'SELECT page_title, page_namespace, page_latest FROM page WHERE page_namespace = 12 AND page_title = '.$dbr->addQuotes($title) );
 62+ $page = $dbr->fetchObject( $page );
6163 }
6264 }
63 - if ( !empty($page->page_title) ) {
64 - $rev = $dbr->select( 'revision',
65 - array( 'rev_id', 'rev_text_id' ),
66 - 'rev_id = '.$dbr->addQuotes($page->page_latest),
67 - __METHOD__ );
68 - $rev = $dbr->fetchObject( $rev );
69 - } else {
70 - return false;
71 - }
72 - $text = $dbr->select( 'text',
73 - array( 'old_id', 'old_text' ),
74 - 'old_id = '.$dbr->addQuotes($rev->rev_text_id),
75 - __METHOD__ );
76 - $text = $dbr->fetchObject( $text );
 65+ }
 66+ if ( !empty( $page->page_title ) ) {
 67+ $rev = $dbr->select( 'revision',
 68+ array( 'rev_id', 'rev_text_id' ),
 69+ array( 'rev_id' => $page->page_latest ),
 70+ __METHOD__
 71+ );
 72+ $rev = $dbr->fetchObject( $rev );
 73+ } else {
 74+ return false;
 75+ }
 76+ $text = $dbr->select( 'text',
 77+ array( 'old_id', 'old_text' ),
 78+ array( 'old_id' => $rev->rev_text_id ),
 79+ __METHOD__
 80+ );
 81+ $text = $dbr->fetchObject( $text );
7782
78 - if ( !empty($text->old_text) ) {
79 - echo $wgOut->addWikiText( $text->old_text );
80 - return true;
81 - } else {
82 - return false;
83 - }
84 -
85 - } else {
86 - return false;
 83+ if ( !empty( $text->old_text ) ) {
 84+ $wgOut->addWikiText( $text->old_text );
 85+ return true;
 86+ } else {
 87+ return false;
8788 }
88 -
8989 }
9090
91 -
92 -function wfHelpCommonsRedirectTalks( &$article, &$outputDone, &$pcache ) {
 91+/**
 92+ * @param $article
 93+ * @param $fields
 94+ * @return bool
 95+ */
 96+function wfHelpCommonsRedirectTalks( $article, $fields ) {
9397 global $wgTitle, $wgOut, $wgContLang, $wgHelpCommonsFetchingWikis, $wgLanguageCode, $wgDBname;
9498
95 - if ( $wgTitle->getNamespace() == NS_HELP_TALK ) {
 99+ if ( $wgTitle->getNamespace() != NS_HELP_TALK ) {
 100+ return false;
 101+ }
96102
97 - $replacewhitespace = str_replace( ' ', '_', $wgOut->getTitle() );
98 - $title = str_replace( $wgContLang->namespaceNames[NS_HELP_TALK].':', '', $replacewhitespace );
 103+ $replacewhitespace = str_replace( ' ', '_', $wgOut->getTitle() );
 104+ $title = str_replace( $wgContLang->namespaceNames[NS_HELP_TALK].':', '', $replacewhitespace );
99105
100 - foreach ( $wgHelpCommonsFetchingWikis as $language => $urls ) {
101 - foreach ( $urls as $url => $wgHelpCommonsFetchingWiki ) {
102 - if ( $wgLanguageCode == "$language" && $wgDBname != $wgHelpCommonsFetchingWiki ) {
103 - $dbr = wfGetDB( DB_SLAVE, array(), $wgHelpCommonsFetchingWiki );
104 - $page = $dbr->query( 'SELECT page_title, page_namespace, page_latest FROM page WHERE page_namespace = 12 AND page_title = '.$dbr->addQuotes($title) );
105 - $page = $dbr->fetchObject( $page );
 106+ foreach ( $wgHelpCommonsFetchingWikis as $language => $urls ) {
 107+ foreach ( $urls as $url => $helpwiki ) {
 108+ if ( $wgLanguageCode == "$language" && $wgDBname != $helpwiki ) {
 109+ $dbr = wfGetDB( DB_SLAVE, array(), $helpwiki );
 110+ $page = $dbr->select(
 111+ 'page',
 112+ array( 'page_title', 'page_namespace', 'page_latest' ),
 113+ array( 'page_namespace' => 12, 'page_title' => $title ),
 114+ __METHOD__
 115+ );
 116+ $page = $dbr->fetchObject( $page );
 117+ }
 118+ if ( !empty( $page->page_title ) ) {
 119+ if ( $page->page_title == $title && !$wgTitle->exists() ) {
 120+ $helpCommonsRedirectTalk = Title::newFromText( $url . '/index.php?title=' . str_replace( ' ', '_', $wgOut->getTitle() ) );
 121+ $redirectTalkPage = $helpCommonsRedirectTalk->getFullText();
 122+ $wgOut->redirect( $redirectTalkPage );
 123+ return true;
 124+ } else {
 125+ return false;
106126 }
107 - if ( !empty($page->page_title) ) {
108 - if ( $page->page_title == $title && !$wgTitle->exists() ) {
109 - $HelpCommonsRedirectTalk = Title::newFromText( $url . '/index.php?title=' . str_replace( ' ', '_', $wgOut->getTitle() ) );
110 - $redirectTalkPage = $HelpCommonsRedirectTalk->getFullText();
111 - $wgOut->redirect( $redirectTalkPage );
112 - return true;
113 - } else {
114 - return false;
115 - }
116 - }
117127 }
118128 }
119 - return true;
120 - } else {
121 - return false;
122129 }
 130+ return true;
123131 }
124132
125 -
 133+/**
 134+ * @param $skin
 135+ * @param $target Title
 136+ * @param $text
 137+ * @param $customAttribs
 138+ * @param $query
 139+ * @param $options array
 140+ * @param $ret
 141+ * @return bool
 142+ */
126143 function efHelpCommonsMakeBlueLinks( $skin, $target, &$text, &$customAttribs, &$query, &$options, &$ret ) {
127144
128145 if ( is_null( $target ) ) {
——@@ -145,60 +162,78 @@
146163 return true;
147164 }
148165
149 -
 166+/**
 167+ * @param $skin
 168+ * @param $title Title
 169+ * @param $section
 170+ * @param $tooltip
 171+ * @param $result
 172+ * @param bool $lang
 173+ * @return bool
 174+ */
150175 function wfHelpCommonsChangeEditSectionLink( $skin, $title, $section, $tooltip, $result, $lang = false ) {
151176 global $wgTitle, $wgHelpCommonsFetchingWikis, $wgLanguageCode, $wgDBname;
152177
153 - if ( $wgTitle->getNamespace() == NS_HELP ) {
154 - foreach ( $wgHelpCommonsFetchingWikis as $language => $urls ) {
155 - foreach ( $urls as $url => $wgHelpCommonsFetchingWiki ) {
156 - if ( $wgLanguageCode == "$language" && $wgDBname != $wgHelpCommonsFetchingWiki ) {
157 - $result = '<span class="editsection">[<a href="'.$url.'/index.php?title='.str_replace( ' ', '_', $title ).'&amp;action=edit&amp;section='.$section.'" title="'.wfMsg( 'editsectionhint', $tooltip ).'">'.wfMsg( 'editsection' ).'</a>]</span>';
158 - }
 178+ if ( $wgTitle->getNamespace() != NS_HELP ) {
 179+ return false;
 180+ }
 181+ foreach ( $wgHelpCommonsFetchingWikis as $language => $urls ) {
 182+ foreach ( $urls as $url => $helpwiki ) {
 183+ if ( $wgLanguageCode == "$language" && $wgDBname != $helpwiki ) {
 184+ // FIXME: $result is unused
 185+ $result = '<span class="editsection">[<a href="' . $url . '/index.php?title=' .
 186+ str_replace( ' ', '_', $title ) . '&amp;action=edit&amp;section=' . $section .
 187+ '" title="' . wfMsg( 'editsectionhint', $tooltip ) . '">' . wfMsg( 'editsection' ) . '</a>]</span>';
159188 }
160189 }
161 - return true;
162 - } else {
163 - return false;
164190 }
165 -
 191+ return true;
166192 }
167193
168 -
 194+/**
 195+ * @param $title Title
 196+ * @param $user User
 197+ * @param $action
 198+ * @param $result
 199+ * @return bool
 200+ */
169201 function fnProtectHelpCommons( &$title, &$user, $action, &$result) {
170202 global $wgHelpCommonsFetchingWikis, $wgDBname;
171 -
172 - foreach ( $wgHelpCommonsFetchingWikis as $language => $urls ) {
173 - foreach ( $urls as $url => $wgHelpCommonsFetchingWiki ) {
174 - // only protect Help pages on non-help-pages-fetching wikis
175 - if( $wgDBname != $wgHelpCommonsFetchingWiki ) {
176203
177 - // block actions 'edit' and 'create'
178 - if( $action != 'edit' && $action != 'create' ) {
179 - return true;
180 - }
 204+ foreach ( $wgHelpCommonsFetchingWikis as $urls ) {
 205+ // FIXME: don't use global esk variable names for non globals
 206+ foreach ( $urls as $url => $helpwiki ) {
 207+ // only protect Help pages on non-help-pages-fetching wikis
 208+ if( $wgDBname != $helpwiki ) {
 209+ // block actions 'edit' and 'create'
 210+ if( $action != 'edit' && $action != 'create' ) {
 211+ return true;
 212+ }
181213
182 - $dbr = wfGetDB(DB_SLAVE, array(), $wgHelpCommonsFetchingWiki);
183 - $res = $dbr->query( 'SELECT page_title, page_namespace FROM page WHERE page_namespace = 12 AND page_title = '.$dbr->addQuotes(str_replace( ' ', '_', $title->getText())) );
 214+ $dbr = wfGetDB( DB_SLAVE, array(), $helpwiki );
 215+ $res = $dbr->select(
 216+ 'page',
 217+ array( 'page_title', 'page_namespace', 'page_latest' ),
 218+ array( 'page_namespace' => 12, 'page_title' => str_replace( ' ', '_', $title->getText() ) ),
 219+ __METHOD__
 220+ );
184221
185 - if ( $dbr->numRows($res) < 1 ) {
186 - return true;
187 - }
 222+ if ( $dbr->numRows( $res ) < 1 ) {
 223+ return true;
 224+ }
188225
189 - $ns = $title->getNamespace();
 226+ $ns = $title->getNamespace();
190227
191 - // check namespaces
192 - if( $ns == 12 || $ns == 13 ) {
 228+ // check namespaces
 229+ if( $ns == 12 || $ns == 13 ) {
 230+ // error message if action is blocked
 231+ $result = array( 'protectedpagetext' );
193232
194 - // error message if action is blocked
195 - $result = array( 'protectedpagetext' );
196 -
197 - // bail, and stop the request
198 - return false;
199 - }
 233+ // bail, and stop the request
 234+ return false;
 235+ }
 236+ }
200237 }
201238 }
202 - }
203 -
204239 return true;
205240 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r104683select instead of querycervidae14:48, 30 November 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r104344Confine DoEditSectionLink Hook to NS_HELPcervidae16:42, 27 November 2011
r104364Bring some of the DB code into this century...reedy21:09, 27 November 2011

Comments

#Comment by Nikerabbit (talk | contribs)   07:07, 30 November 2011

Looks okay. Some of those FIXMEs can now be removed, right?

#Comment by SVG (talk | contribs)   14:51, 30 November 2011

Sure. Reedy fixed all of them.

Status & tagging log