r90650 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r90649‎ | r90650 | r90651 >
Date:06:44, 23 June 2011
Author:wikinaut
Status:deferred (Comments)
Tags:
Comment:
new version 1.6.3; adds edit count of fromUser to toUser; move; redirects user page of fromUser to toUser, if this does not exist or is empty; deletes fromUser page if toUser page exists; code contributed by Matthew April. small changes and checked by committer
Modified paths:
  • /trunk/extensions/UserMerge/UserMerge.i18n.php (modified) (history)
  • /trunk/extensions/UserMerge/UserMerge.php (modified) (history)
  • /trunk/extensions/UserMerge/UserMerge_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/UserMerge/UserMerge_body.php
@@ -3,15 +3,15 @@
44 * \brief Contains code for the UserMerge Class (extends SpecialPage).
55 */
66
7 -///Special page class for the User Merge and Delete extension
87 /**
9 - * Special page that allows sysops to merge references from one
10 - * user to another user - also supports deleting users following
11 - * merge.
 8+ * Special page class for the User Merge and Delete extension
 9+ * allows sysops to merge references from one user to another user.
 10+ * It also supports deleting users following merge.
1211 *
1312 * @ingroup Extensions
1413 * @author Tim Laqua <t.laqua@gmail.com>
1514 */
 15+
1616 class UserMerge extends SpecialPage {
1717 function __construct() {
1818 parent::__construct( 'UserMerge', 'usermerge' );
@@ -155,16 +155,19 @@
156156 $wgOut->addHTML( "<span style=\"color: red;\">" . wfMsg( 'usermerge-badtoken' ) . "</span><br />\n" );
157157 } else {
158158 //good editToken
 159+ $this->mergeEditcount( $newuserID,$olduserID );
159160 $this->mergeUser( $objNewUser, $newuser_text, $newuserID, $objOldUser, $olduser_text, $olduserID );
160161 if ( $wgRequest->getText( 'deleteuser' ) ) {
 162+ $this->movePages( $newuser_text, $olduser_text );
161163 $this->deleteUser( $objOldUser, $olduserID, $olduser_text);
162164 }
163165 }
164166 }
165167 }
166168
167 - ///Function to delete users following a successful mergeUser call
168169 /**
 170+ * Function to delete users following a successful mergeUser call
 171+ *
169172 * Removes user entries from the user table and the user_groups table
170173 *
171174 * @param $olduserID int ID of user to delete
@@ -193,8 +196,10 @@
194197 return true;
195198 }
196199
197 - ///Function to merge database references from one user to another user
 200+
198201 /**
 202+ * Function to merge database references from one user to another user
 203+ *
199204 * Merges database references from one user ID or username to another user ID or username
200205 * to preserve referential integrity.
201206 *
@@ -255,4 +260,153 @@
256261
257262 return true;
258263 }
 264+
 265+ ///Function to add edit count
 266+ /**
 267+ * Adds edit count of both users
 268+ *
 269+ * @param $newuserID int ID of user to merge references TO
 270+ * @param $olduserID int ID of user to remove references FROM
 271+ *
 272+ * @return Always returns true - throws exceptions on failure.
 273+ *
 274+ * @author Matthew April <Matthew.April@tbs-sct.gc.ca>
 275+ */
 276+ private function mergeEditcount( $newuserID, $olduserID ) {
 277+ global $wgOut;
 278+
 279+ $dbw =& wfGetDB( DB_MASTER );
 280+
 281+ # old user edit count
 282+ $result = $dbw->select( 'user', array('user_editcount'), 'user_id ='.$olduserID );
 283+ $row = $dbw->fetchRow($result);
 284+ $oldEdits = $row[0];
 285+
 286+ # new user edit count
 287+ $result = $dbw->select( 'user', array('user_editcount'), 'user_id ='.$newuserID );
 288+ $row = $dbw->fetchRow($result);
 289+ $newEdits = $row[0];
 290+
 291+ # add edits
 292+ $totalEdits = $oldEdits + $newEdits;
 293+
 294+ # don't run querys if neither user has any edits
 295+ if( $totalEdits > 0 ) {
 296+ # update new user with total edits
 297+ $dbw->update( 'user', array('user_editcount' => $totalEdits), array('user_id' => $newuserID) );
 298+
 299+ #clear old users edits
 300+ $dbw->update( 'user', array('user_editcount' => 0), array('user_id' => $olduserID) );
 301+ }
 302+
 303+ $wgOut->addHTML(wfMsgForContent('usermerge-editcount-success', $olduserID, $newuserID) . "<br />\n");
 304+
 305+ return true;
 306+ }
 307+
 308+
 309+ /**
 310+ * Function to merge user pages
 311+ *
 312+ * Deletes all pages when merging to Anon
 313+ * Moves user page when the target user page does not exist or is empty
 314+ * Deletes redirect if nothing links to old page
 315+ * Deletes the old user page when the target user page exists
 316+ *
 317+ * @param $newuser_text string Username to merge pages TO
 318+ * @param $olduser_text string Username of user to remove pages FROM
 319+ *
 320+ * @return returns true on completion
 321+ *
 322+ * @author Matthew April <Matthew.April@tbs-sct.gc.ca>
 323+ */
 324+ private function movePages( $newuser_text, $olduser_text ) {
 325+
 326+ global $wgOut, $wgTitle, $wgContLang, $wgUser;
 327+
 328+ $oldusername = trim( str_replace( '_', ' ', $olduser_text ) );
 329+ $oldusername = Title::makeTitle( NS_USER, $oldusername );
 330+ $newusername = Title::makeTitleSafe( NS_USER, $wgContLang->ucfirst( $newuser_text ) );
 331+
 332+ # select all user pages and sub-pages
 333+ $dbr = wfGetDB( DB_SLAVE );
 334+ $oldkey = $oldusername->getDBkey();
 335+ $pages = $dbr->select(
 336+ 'page',
 337+ array( 'page_namespace', 'page_title' ),
 338+ array(
 339+ 'page_namespace IN (' . NS_USER . ',' . NS_USER_TALK . ')',
 340+ '(page_title LIKE ' .
 341+ $dbr->addQuotes( $dbr->escapeLike( $oldusername->getDBkey() ) . '/%' ) .
 342+ ' OR page_title = ' . $dbr->addQuotes( $oldusername->getDBkey() ) . ')'
 343+ )
 344+ );
 345+
 346+ $output = '';
 347+ $skin =& $wgUser->getSkin();
 348+ while ( $row = $dbr->fetchObject( $pages ) ) {
 349+ $oldPage = Title::makeTitleSafe( $row->page_namespace, $row->page_title );
 350+ $newPage = Title::makeTitleSafe( $row->page_namespace,
 351+ preg_replace( '!^[^/]+!', $newusername->getDBkey(), $row->page_title ) );
 352+
 353+
 354+ if( $newuser_text == "Anonymous" ) { # delete ALL old pages
 355+
 356+ if( $oldPage->exists() ) {
 357+ $oldPageArticle = new Article($oldPage);
 358+ $oldPageArticle->doDeleteArticle( wfMsgHtml('usermerge-autopagedelete') );
 359+
 360+ $oldLink = $skin->makeKnownLinkObj( $oldPage );
 361+ $output .= '<li class="mw-renameuser-pe">' . wfMsgHtml( 'usermerge-page-deleted', $oldLink ) . '</li>';
 362+ }
 363+
 364+ } elseif( $newPage->exists() && !$oldPage->isValidMoveTarget( $newPage ) && $newPage->getLength() > 0) { # delete old pages that can't be moved
 365+
 366+ $oldPageArticle = new Article($oldPage);
 367+ $oldPageArticle->doDeleteArticle( wfMsgHtml('usermerge-autopagedelete') );
 368+
 369+ $link = $skin->makeKnownLinkObj( $oldPage );
 370+ $output .= '<li class="mw-renameuser-pe">' . wfMsgHtml( 'usermerge-page-deleted', $link ) . '</li>';
 371+
 372+ } else { # move content to new page
 373+
 374+ # delete target page if it exists and is blank
 375+ if( $newPage->exists() ) {
 376+ $newPageArticle = new Article($newPage);
 377+ $newPageArticle->doDeleteArticle('usermerge-autopagedelete');
 378+ }
 379+
 380+ # move to target location
 381+ $success = $oldPage->moveTo( $newPage, false, wfMsgForContent( 'usermerge-move-log',
 382+ $oldusername->getText(), $newusername->getText() ) );
 383+ if( $success === true ) {
 384+ $oldLink = $skin->makeKnownLinkObj( $oldPage, '', 'redirect=no' );
 385+ $newLink = $skin->makeKnownLinkObj( $newPage );
 386+ $output .= '<li class="mw-renameuser-pm">' . wfMsgHtml( 'usermerge-page-moved', $oldLink, $newLink ) . '</li>';
 387+ } else {
 388+ $oldLink = $skin->makeKnownLinkObj( $oldPage );
 389+ $newLink = $skin->makeLinkObj( $newPage );
 390+ $output .= '<li class="mw-renameuser-pu">' . wfMsgHtml( 'usermerge-page-unmoved', $oldLink, $newLink ) . '</li>';
 391+ }
 392+
 393+ # check if any pages link here
 394+ $res = $dbr->select( 'pagelinks',
 395+ 'pl_title' ,
 396+ array( 'pl_title' => $olduser_text ),
 397+ __METHOD__
 398+ );
 399+ if( !$dbr->numRows( $res ) ) {
 400+ # nothing links here, so delete unmoved page/redirect
 401+ $oldPageArticle = new Article($oldPage);
 402+ $oldPageArticle->doDeleteArticle( wfMsgHtml('usermerge-autopagedelete') );
 403+ }
 404+
 405+ }
 406+ }
 407+
 408+ if( $output )
 409+ $wgOut->addHTML( '<ul>' . $output . '</ul>' );
 410+
 411+ return true;
 412+ }
259413 }
Index: trunk/extensions/UserMerge/UserMerge.i18n.php
@@ -33,6 +33,13 @@
3434 'usermerge-unmergable' => 'Unable to merge from user - ID or name has been defined as unmergable.',
3535 'usermerge-protectedgroup' => 'Unable to merge from user - user is in a protected group.',
3636 'right-usermerge' => 'Merge users',
 37+ 'usermerge-editcount' => 'Add edit count?',
 38+ 'usermerge-editcount-success' => 'Adding edit count of ($1 and $2)',
 39+ 'usermerge-autopagedelete' => 'Automatically deleted when merging users',
 40+ 'usermerge-page-unmoved' => 'The page $1 could not be moved to $2.',
 41+ 'usermerge-page-moved' => 'The page $1 has been moved to $2.',
 42+ 'usermerge-move-log' => 'Automatically moved page while merging the user "[[User:$1|$1]]" to "[[User:$2|$2]]"',
 43+ 'usermerge-page-deleted' => 'Deleted page $1',
3744 );
3845
3946 /** Message documentation (Message documentation)
Index: trunk/extensions/UserMerge/UserMerge.php
@@ -3,6 +3,27 @@
44 * \brief Contains setup code for the User Merge and Delete Extension.
55 */
66
 7+/*
 8+ * UserMerge Extension for MediaWiki
 9+ *
 10+ * Copyright (C) PediaPress GmbH
 11+ *
 12+ * This program is free software; you can redistribute it and/or modify
 13+ * it under the terms of the GNU General Public License as published by
 14+ * the Free Software Foundation; either version 2 of the License, or
 15+ * (at your option) any later version.
 16+ *
 17+ * This program is distributed in the hope that it will be useful,
 18+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
 19+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 20+ * GNU General Public License for more details.
 21+ *
 22+ * You should have received a copy of the GNU General Public License along
 23+ * with this program; if not, write to the Free Software Foundation, Inc.,
 24+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 25+ * http://www.gnu.org/copyleft/gpl.html
 26+ */
 27+
728 # Not a valid entry point, skip unless MEDIAWIKI is defined
829 if (!defined('MEDIAWIKI')) {
930 echo "User Merge and Delete extension";
@@ -13,9 +34,9 @@
1435 'path' => __FILE__,
1536 'name' => 'User Merge and Delete',
1637 'url' => 'http://www.mediawiki.org/wiki/Extension:User_Merge_and_Delete',
17 - 'author' => array( 'Tim Laqua', 'Thomas Gries' ),
 38+ 'author' => array( 'Tim Laqua', 'Thomas Gries', 'Matthew April' ),
1839 'descriptionmsg' => 'usermerge-desc',
19 - 'version' => '1.6.21'
 40+ 'version' => '1.6.3'
2041 );
2142
2243 $wgAvailableRights[] = 'usermerge';

Follow-up revisions

RevisionCommit summaryAuthorDate
r90684follow up patch to r90650 ; making most of it conforming to mw coding convent...wikinaut00:11, 24 June 2011
r90854correction of the copyright message, which was incorrectly introduced in r90650wikinaut20:41, 26 June 2011

Comments

#Comment by Wikinaut (talk | contribs)   06:51, 23 June 2011

based on Extension Update (May 2011) [FIXED in SVN]:

Extension Update (May 2011) [FIXED in SVN]

committed in [[rev:90650|r90650]] --Wikinaut 06:48, 23 June 2011 (UTC)

I've been working on an update for this extension which I will hopefully be releasing shortly. It adds support for:

  • Merging user watchlists  OK done. [[Special:Code/MediaWiki/89015|r89015]] --Wikinaut 03:00, 28 May 2011 (UTC)
  • Adding user edit count  OK [[Special:Code/MediaWiki/90650|r90650]] --Wikinaut 06:48, 23 June 2011 (UTC)
  • Moving user pages (on user deletion)  OK [[Special:Code/MediaWiki/90650|r90650]] --Wikinaut 06:48, 23 June 2011 (UTC)

However, I will only be able to test this updated version on MW 1.16 and maybe 1.13. If anyone is willing to test it on other versions before the release please let me know. --Matthew.JA 14:19, 2 May 2011 (UTC)

#Comment by Wikinaut (talk | contribs)   06:56, 23 June 2011

provisionally flagged as fixme, because of the major changes it may introduce (redirect and delete user page of merged fromUser).

#Comment by Nikerabbit (talk | contribs)   08:21, 23 June 2011

This is remnant from PHP4 time, don't use =& but just =.

$dbw =& wfGetDB( DB_MASTER );

You can use selectField.

$result = $dbw->select( 'user', array('user_editcount'), 'user_id ='.$olduserID );
$row = $dbw->fetchRow($result);

Use array( 'user_id' => $newUserID ), it will do escaping for you.

'user_id ='.$newuserID

Message output should be html escaped or parsed:

$wgOut->addHTML(wfMsgForContent('usermerge-editcount-success', $olduserID, $newuserID) . "
\n");

escapeLike is deprecated, use buildLike and anyString.

$dbr->addQuotes( $dbr->escapeLike( $oldusername->getDBkey() ) . '/%' ) .

Don't use fetchObject, use foreach ( $pages as $row )

while ( $row = $dbr->fetchObject( $pages ) ) {

I'd prefer strict === comparison here

if( $newuser_text == "Anonymous" ) { # delete ALL old pages

Must be new Article( $oldPage, 0 )

$oldPageArticle = new Article($oldPage);

makeKnownLinkObj is deprecated, use link or linkKnown

$oldLink = $skin->makeKnownLinkObj( $oldPage );
#Comment by Wikinaut (talk | contribs)   09:01, 23 June 2011

ok, will be done this evening at the latest. I should have better invested more time, and will consider anything you said for all me forthcoming commits, I promise.

#Comment by Wikinaut (talk | contribs)   00:14, 24 June 2011

everything fixed like suggested, (learnt a lot)

  • except $wgOut->addHTML(wfMsgForContent('usermerge-editcount-success', $olduserID, $newuserID) . "

\n");

  • should be done together with the others addHTMLs
#Comment by Wikinaut (talk | contribs)   00:26, 24 June 2011

I tag this a "resolved" because it is fixed by the r90684 , which is now "fixme". This is what I understood from the CR Guide.

#Comment by Wikinaut (talk | contribs)   00:28, 24 June 2011

( pls. correct me if I wasn't entitled to do _that_ )

#Comment by Aaron Schulz (talk | contribs)   02:27, 24 June 2011

Never resolve or OK your own revisions. If you made a fix to rev X in a newer revision, say rev Y, then just reset rev X to "new" and let someone else OK/resolve it. Be sure to mention rev X in rev Y (as you already did).

#Comment by Wikinaut (talk | contribs)   05:37, 24 June 2011

Aaron, thanks. Now I found a corresponding warning back - on the "Code review" page. It was not present on the longer "Code review guide" which I consulted. I changed the page so that now both pages include the status flag table, and this warning. Remark: both pages are also now part of a new scetion "Code review" of the book http://www.mediawiki.org/wiki/MDG (MW Developer's Guide).

Reset (own) revision "new", but fixed in the follow-up r90684 ("fixme").

#Comment by Wikinaut (talk | contribs)   06:12, 24 June 2011

an alternative would be to fully revert this revision r90650 to make CR easierer

#Comment by OverlordQ (talk | contribs)   07:06, 24 June 2011

Does it really matter for extensions?

Or are we forcing MW dev rules on everything that's in svn?

#Comment by P858snake (talk | contribs)   07:09, 24 June 2011

Code review is the same for everything that is getting reviewed (the only exception really deferred for non wmf running extensions, or if you revert yourself.)

#Comment by Wikinaut (talk | contribs)   08:20, 24 June 2011

I think so (see Peacheys comment). As a (feel-like) responsible maintainer of extensions, I also do not want to break anything, and many extensions are part of the WMF wikis, so I think they should be fully inline with CC, also to make security issues easy to detect and to be fixed quickly, of course.

#Comment by Wikinaut (talk | contribs)   08:22, 24 June 2011

The "Copyright (C) PediaPress Gmbh" was mistakenly added by myself when copying that from "Extension:Collection" and should be removed soon. Will do in a couple of hours.

Status & tagging log