r73415 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r73414‎ | r73415 | r73416 >
Date:21:56, 20 September 2010
Author:churchofemacs
Status:ok (Comments)
Tags:
Comment:
fixing r73376: error with IP userpages
Modified paths:
  • /trunk/extensions/Renameuser/Renameuser.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Renameuser/Renameuser.php
——@@ -72,7 +72,7 @@
7373 global $wgOut;
7474 $title = $article->getTitle();
7575 $oldUserName = User::newFromName( $title->getBaseText() );
76 - if ( ($title->getNamespace() == NS_USER || $title->getNamespace() == NS_USER_TALK ) && $oldUserName->getId()==0) {
 76+ if ( ($title->getNamespace() == NS_USER || $title->getNamespace() == NS_USER_TALK ) && ($oldUserName && $oldUserName->getId()==0 )) {
7777 // Get the title for the base userpage
7878 $page = Title::makeTitle( NS_USER, str_replace( ' ', '_', $title->getBaseText() ) )->getPrefixedDBkey();
7979 LogEventsList::showLogExtract( $wgOut, 'renameuser', $page, '', array( 'lim' => 10, 'showIfEmpty' => false,

Follow-up revisions

RevisionCommit summaryAuthorDate
r73531cosmetic code changes to r73415 and r73376churchofemacs14:00, 22 September 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r73376don't display renameuserlog on pages of usurped accounts. Thanks to Chzz for ...churchofemacs14:29, 20 September 2010

Comments

#Comment by Nikerabbit (talk | contribs)   08:05, 21 September 2010

The variable name is misleading. I would have expected it to be a string because of the Name part. There should be spaces around ==. There is also User::isAnon(). Why is the check there anyway? Can it not be that someone else has been renamed to take the previously made free name?

#Comment by Church of emacs (talk | contribs)   13:51, 22 September 2010

I'm not sure how I should use isAnon(). It seems to be a non-static function, so I'd have to create a user object first. However, User::newFromName returns false if it's an IP.

The reason why there should not be a message in userspace X if user X exists, is that this userspace belongs to X. It does not belong to formerlyKnownAsX. As long as X does not exist, the userspace is free, and so we can display the message.

On Wikimedia wikis specifically, there's a problem with usurped accounts. This message should not be displayed, as it is in Keegan's userspace. http://en.wikipedia.org/wiki/User:Keegan/NonExistingPage

#Comment by Nikerabbit (talk | contribs)   13:54, 22 September 2010

Like Roan said, isAnon() is just replacement for getId() == 0. Although in this case it's not much better. I think there should User::exists().

#Comment by Church of emacs (talk | contribs)   13:53, 22 September 2010

I will fix the other problems you mentioned after we figure out what to do with this proposed change. :)

#Comment by Church of emacs (talk | contribs)   14:01, 22 September 2010

Done in r73531.

#Comment by Catrope (talk | contribs)   13:53, 22 September 2010

isAnon() is equivalent to getID() == 0

Status & tagging log