r41435 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r41434‎ | r41435 | r41436 >
Date:18:24, 30 September 2008
Author:aaron
Status:old (Comments)
Tags:partially reverted 
Comment:
* Add option to reserve old name (bug 15181)
* Use LogEventsList::showLogExtract
* Remove now-redundant code
* Break some long lines
* Code space tweaks
Modified paths:
  • /trunk/extensions/Renameuser/SpecialRenameuser.i18n.php (modified) (history)
  • /trunk/extensions/Renameuser/SpecialRenameuser_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Renameuser/SpecialRenameuser_body.php
——@@ -31,30 +31,33 @@
3232 global $wgVersion, $wgMaxNameChars, $wgCapitalLinks;
3333
3434 $this->setHeaders();
35 -
3635 $wgOut->addWikiMsg( 'renameuser-summary' );
3736
38 - if ( !$wgUser->isAllowed( 'renameuser' ) ) {
 37+ if( !$wgUser->isAllowed( 'renameuser' ) ) {
3938 $wgOut->permissionRequired( 'renameuser' );
4039 return;
4140 }
42 -
43 - if ( wfReadOnly() ) {
 41+ if( wfReadOnly() ) {
4442 $wgOut->readOnlyPage();
4543 return;
4644 }
4745
4846 $showBlockLog = $wgRequest->getBool( 'submit-showBlockLog' );
49 - $oldusername = Title::makeTitle( NS_USER, $wgRequest->getText( 'oldusername' ) );
50 - // Force uppercase of newusername otherweise wikis with wgCapitalLinks=false can create lc usernames
 47+ $oldusername = Title::makeTitle( NS_USER, trim( $wgRequest->getText( 'oldusername' ) ) );
 48+ // Force uppercase of newusername, otherwise wikis with wgCapitalLinks=false can create lc usernames
5149 $newusername = Title::newFromText( $wgContLang->ucfirst( $wgRequest->getText( 'newusername' ) ), NS_USER );
5250 $oun = is_object( $oldusername ) ? $oldusername->getText() : '';
5351 $nun = is_object( $newusername ) ? $newusername->getText() : '';
5452 $token = $wgUser->editToken();
5553 $reason = $wgRequest->getText( 'reason' );
56 - $is_checked = true;
57 - if ( $wgRequest->wasPosted() && ! $wgRequest->getCheck( 'movepages' ) ) {
58 - $is_checked = false;
 54+ // If nothing given for these flags, assume they are checked
 55+ // unless this is a POST submission.
 56+ $move_checked = $reserve_checked = true;
 57+ if( $wgRequest->wasPosted() ) {
 58+ if( !$wgRequest->getCheck( 'movepages' ) )
 59+ $move_checked = false;
 60+ if( !$wgRequest->getCheck( 'reservename' ) )
 61+ $reserve_checked = false;
5962 }
6063 $warnings = array();
6164 if( $oun && $nun && !$wgRequest->getCheck( 'confirmaction' ) ) {
——@@ -92,17 +95,28 @@
9396 "</td>
9497 </tr>"
9598 );
96 - if ( $wgUser->isAllowed( 'move' ) ) {
 99+ if( $wgUser->isAllowed( 'move' ) ) {
97100 $wgOut->addHTML( "
98101 <tr>
99102 <td>&nbsp;
100103 </td>
101104 <td class='mw-input'>" .
102 - Xml::checkLabel( wfMsg( 'renameusermove' ), 'movepages', 'movepages', $is_checked, array( 'tabindex' => '4' ) ) .
 105+ Xml::checkLabel( wfMsg( 'renameusermove' ), 'movepages', 'movepages',
 106+ $move_checked, array( 'tabindex' => '4' ) ) .
103107 "</td>
104108 </tr>"
105109 );
106110 }
 111+ $wgOut->addHTML( "
 112+ <tr>
 113+ <td>&nbsp;
 114+ </td>
 115+ <td class='mw-input'>" .
 116+ Xml::checkLabel( wfMsg( 'renameuserreserve' ), 'reservename', 'reservename',
 117+ $reserve_checked, array( 'tabindex' => '4' ) ) .
 118+ "</td>
 119+ </tr>"
 120+ );
107121 if( $warnings ) {
108122 $warningsHtml = array();
109123 foreach( $warnings as $warning )
——@@ -114,7 +128,8 @@
115129 <td>".wfMsgWikiHtml( 'renameuserwarnings' ) ."
116130 </td>
117131 <td class='mw-input'>" .
118 - '<ul style="color: red; font-weight: bold"><li>'.implode( '</li><li>', $warningsHtml ).'</li></ul>'.
 132+ '<ul style="color: red; font-weight: bold"><li>'.
 133+ implode( '</li><li>', $warningsHtml ).'</li></ul>'.
119134 "</td>
120135 </tr>"
121136 );
——@@ -123,18 +138,19 @@
124139 <td>&nbsp;
125140 </td>
126141 <td class='mw-input'>" .
127 - Xml::checkLabel( wfMsg( 'renameuserconfirm' ), 'confirmaction', 'confirmaction', false, array( 'tabindex' => '5' ) ) .
 142+ Xml::checkLabel( wfMsg( 'renameuserconfirm' ), 'confirmaction', 'confirmaction',
 143+ false, array( 'tabindex' => '5' ) ) .
128144 "</td>
129145 </tr>"
130146 );
131147 }
132 -
133148 $wgOut->addHTML( "
134149 <tr>
135150 <td>&nbsp;
136151 </td>
137152 <td class='mw-submit'>" .
138 - Xml::submitButton( wfMsg( 'renameusersubmit' ), array( 'name' => 'submit', 'tabindex' => '6', 'id' => 'submit' ) ) .
 153+ Xml::submitButton( wfMsg( 'renameusersubmit' ), array( 'name' => 'submit',
 154+ 'tabindex' => '6', 'id' => 'submit' ) ) .
139155 ' ' .
140156 Xml::submitButton( wfMsg( 'blocklogpage' ), array ( 'name' => 'submit-showBlockLog',
141157 'id' => 'submit-showBlockLog', 'tabindex' => '7' ) ) .
——@@ -147,8 +163,8 @@
148164 );
149165
150166 // Show block log if requested
151 - if ( $showBlockLog && is_object( $oldusername ) ) {
152 - $this->showLogExtract ( $oldusername, 'block', $wgOut ) ;
 167+ if( $showBlockLog && is_object( $oldusername ) ) {
 168+ $this->showLogExtract( $oldusername, 'block', $wgOut ) ;
153169 return;
154170 }
155171
——@@ -185,50 +201,47 @@
186202 $newuser = User::newFromName( $newusername->getText() );
187203
188204 // It won't be an object if for instance "|" is supplied as a value
189 - if ( !is_object( $olduser ) ) {
190 - $wgOut->addWikiText( "<div class=\"errorbox\">" . wfMsg( 'renameusererrorinvalid', $oldusername->getText() ) . "</div>" );
 205+ if( !is_object( $olduser ) ) {
 206+ $wgOut->addWikiText( "<div class=\"errorbox\">" . wfMsg( 'renameusererrorinvalid',
 207+ $oldusername->getText() ) . "</div>" );
191208 return;
192209 }
193 -
194 - if ( !is_object( $newuser ) || !User::isCreatableName( $newuser->getName() ) ) {
195 - $wgOut->addWikiText( "<div class=\"errorbox\">" . wfMsg( 'renameusererrorinvalid', $newusername->getText() ) . "</div>" );
 210+ if( !is_object( $newuser ) || !User::isCreatableName( $newuser->getName() ) ) {
 211+ $wgOut->addWikiText( "<div class=\"errorbox\">" . wfMsg( 'renameusererrorinvalid',
 212+ $newusername->getText() ) . "</div>" );
196213 return;
197214 }
198215
199216 // Check for the existence of lowercase oldusername in database.
200217 // Until r19631 it was possible to rename a user to a name with first character as lowercase
201 - if ( $wgRequest->getText( 'oldusername' ) !== $wgContLang->ucfirst( $wgRequest->getText( 'oldusername' ) ) ) {
 218+ if( $oldusername->getText() !== $wgContLang->ucfirst( $oldusername->getText() ) ) {
202219 // oldusername was entered as lowercase -> check for existence in table 'user'
203 - $dbr_lc = wfGetDB( DB_SLAVE );
204 - $s = trim( $wgRequest->getText( 'oldusername' ) );
205 - $uid = $dbr_lc->selectField( 'user', 'user_id', array( 'user_name' => $s ), __METHOD__ );
206 - if ( $uid === false ) {
207 - if ( !$wgCapitalLinks ) {
 220+ $dbr = wfGetDB( DB_SLAVE );
 221+ $uid = $dbr->selectField( 'user', 'user_id',
 222+ array( 'user_name' => $oldusername->getText() ),
 223+ __METHOD__ );
 224+ if( $uid === false ) {
 225+ if( !$wgCapitalLinks ) {
208226 $uid = 0; // We are on a lowercase wiki but lowercase username does not exists
209227 } else {
210228 $uid = $olduser->idForName(); // We are on a standard uppercase wiki, use normal
211229 $oldusername = Title::newFromText( $olduser->getName(), NS_USER ); // uppercase form
212230 }
213 - } else {
214 - // username with lowercase exists
215 - // Title::newFromText was nice, but forces uppercase
216 - // for older rename accidents on lowercase wikis we need the lowercase username as entered in the form
217 - $oldusername->mTextform = $wgRequest->getText( 'oldusername' );
218 - $oldusername->mUrlform = $wgRequest->getText( 'oldusername' );
219 - $oldusername->mDbkeyform = $wgRequest->getText( 'oldusername' );
220231 }
221232 } else {
222233 // oldusername was entered as upperase -> standard procedure
223234 $uid = $olduser->idForName();
224235 }
225236
226 - if ($uid == 0) {
227 - $wgOut->addWikiText( "<div class=\"errorbox\">" . wfMsg( 'renameusererrordoesnotexist' , $oldusername->getText() ) . "</div>" );
 237+ if( $uid == 0 ) {
 238+ $wgOut->addWikiText( "<div class=\"errorbox\">" . wfMsg( 'renameusererrordoesnotexist' ,
 239+ $oldusername->getText() ) . "</div>" );
228240 return;
229241 }
230242
231 - if ($newuser->idForName() != 0) {
232 - $wgOut->addWikiText( "<div class=\"errorbox\">" . wfMsg( 'renameusererrorexists', $newusername->getText() ) . "</div>" );
 243+ if( $newuser->idForName() != 0 ) {
 244+ $wgOut->addWikiText( "<div class=\"errorbox\">" . wfMsg( 'renameusererrorexists',
 245+ $newusername->getText() ) . "</div>" );
233246 return;
234247 }
235248
——@@ -236,7 +249,7 @@
237250 $contribs = User::edits( $uid );
238251
239252 // Check edit count
240 - if ( !$wgUser->isAllowed( 'siteadmin' ) ) {
 253+ if( !$wgUser->isAllowed( 'siteadmin' ) ) {
241254 if ( RENAMEUSER_CONTRIBLIMIT != 0 && $contribs > RENAMEUSER_CONTRIBLIMIT ) {
242255 $wgOut->addWikiText( "<div class=\"errorbox\">" .
243256 wfMsg( 'renameusererrortoomany',
——@@ -254,6 +267,7 @@
255268 return;
256269 }
257270
 271+ // Do the heavy lifting...
258272 $rename = new RenameuserSQL( $oldusername->getText(), $newusername->getText(), $uid );
259273 if( !$rename->rename() ) {
260274 return;
——@@ -261,19 +275,24 @@
262276
263277 // If this user is renaming his/herself, make sure that Title::moveTo()
264278 // doesn't make a bunch of null move edits under the old name!
265 - global $wgUser;
266279 if( $wgUser->getId() == $uid ) {
267280 $wgUser->setName( $newusername->getText() );
268281 }
269282
 283+ // Log this rename
270284 $log = new LogPage( 'renameuser' );
271285 $log->addEntry( 'renameuser', $oldusername, wfMsgExt( 'renameuser-log', array( 'parsemag', 'content' ),
272286 $wgContLang->formatNum( $contribs ), $reason ), $newusername->getText() );
273287
274 - $wgOut->addWikiText( "<div class=\"successbox\">" . wfMsg( 'renameusersuccess', $oldusername->getText(),
275 - $newusername->getText() ) . "</div><br style=\"clear:both\" />" );
 288+ // Reserve the old name with a random password
 289+ if( $wgRequest->getCheck( 'reservename' ) ) {
 290+ $p = User::randomPassword();
 291+ $user = User::createNew( $olduser->getName() );
 292+ $user->setNewpassword( $p );
 293+ }
276294
277 - if ( $wgRequest->getCheck( 'movepages' ) && $wgUser->isAllowed( 'move' ) ) {
 295+ // Move any user pages
 296+ if( $wgRequest->getCheck( 'movepages' ) && $wgUser->isAllowed( 'move' ) ) {
278297 $dbr = wfGetDB( DB_SLAVE );
279298 $oldkey = $oldusername->getDBkey();
280299 $pages = $dbr->select(
——@@ -315,22 +334,16 @@
316335 if( $output )
317336 $wgOut->addHtml( '<ul>' . $output . '</ul>' );
318337 }
 338+
 339+ // Output success message stuff :)
 340+ $wgOut->addWikiText( "<div class=\"successbox\">" . wfMsg( 'renameusersuccess', $oldusername->getText(),
 341+ $newusername->getText() ) . "</div><br style=\"clear:both\" />" );
319342 }
320343
321 - // FIXME: this code is total crap. Should this just use LogEventsList or
322 - // since extensions are branched, or are we keeping the half-ass b/c thing?
323344 function showLogExtract( $username, $type, &$out ) {
324 - global $wgOut;
325345 # Show relevant lines from the logs:
326 - $wgOut->addHtml( Xml::element( 'h2', null, LogPage::logName( $type ) ) . "\n" );
327 -
328 - $logViewer = new LogViewer(
329 - new LogReader(
330 - new FauxRequest(
331 - array( 'page' => $username->getPrefixedText(),
332 - 'type' => $type ) ) ) );
333 - $logViewer->showList( $out );
334 -
 346+ $out->addHtml( Xml::element( 'h2', null, LogPage::logName( $type ) ) . "\n" );
 347+ LogEventsList::showLogExtract( $out, $type, $username->getPrefixedText() );
335348 }
336349 }
337350
Index: trunk/extensions/Renameuser/SpecialRenameuser.i18n.php
——@@ -15,6 +15,7 @@
1616 'renameusernew' => 'New username:',
1717 'renameuserreason' => 'Reason for rename:',
1818 'renameusermove' => 'Move user and talk pages (and their subpages) to new name',
 19+ 'renameuserreserve' => 'Reserve the old username from further use',
1920 'renameuserwarnings' => 'Warnings:',
2021 'renameuserconfirm' => 'Yes, rename the user',
2122 'renameusersubmit' => 'Submit',

Follow-up revisions

RevisionCommit summaryAuthorDate
r42756Roll back part of r41435 and following "* Add option to reserve old name (bug......brion01:36, 29 October 2008

Comments

#Comment by Raymond (talk | contribs)   06:24, 1 October 2008

The tabindex 4 is used twice:

+ $move_checked, array( 'tabindex' => '4' ) ) .

+ $reserve_checked, array( 'tabindex' => '4' ) ) .

#Comment by Voice of All (talk | contribs)   00:29, 3 October 2008

Fixed in r41583

#Comment by Siebrand (talk | contribs)   16:44, 7 October 2008

Message wording updated in r41802.

#Comment by Brion VIBBER (talk | contribs)   01:36, 29 October 2008

Rolled back the username reservation in r42756, this has issues (see comment there).

Status & tagging log