r55918 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r55917‎ | r55918 | r55919 >
Date:12:04, 7 September 2009
Author:churchofemacs
Status:resolved (Comments)
Tags:
Comment:
fixing r55909: checking if the user is _currently_ blocked before displaying block log (on Special:Contributions)
Modified paths:
  • /trunk/phase3/includes/specials/SpecialContributions.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialContributions.php
——@@ -223,7 +223,7 @@
224224 wfRunHooks( 'ContributionsToolLinks', array( $id, $nt, &$tools ) );
225225
226226 $links = $wgLang->pipeList( $tools );
227 - $this->showBlock( $nt );
 227+ $this->showBlock( $nt, $id );
228228 }
229229
230230 // Old message 'contribsub' had one parameter, but that doesn't work for
——@@ -241,8 +241,10 @@
242242 * Show a note if the user is blocked and display the last block log entry.
243243 * @param Title $nt Title object for the target
244244 */
245 - protected function showBlock( $nt ) {
 245+ protected function showBlock( $nt, $id ) {
246246 global $wgUser, $wgOut;
 247+ if ( !User::newFromID( $id )->isBlocked() )
 248+ return; # User is not blocked, nothing to do here
247249 $loglist = new LogEventsList( $wgUser->getSkin(), $wgOut );
248250 $pager = new LogPager( $loglist, 'block', false, $nt->getPrefixedText() );
249251 // Check if there is something in the block log.

Follow-up revisions

RevisionCommit summaryAuthorDate
r55982Minor fixes to r55918 - documentation and variable nameschurchofemacs17:49, 7 September 2009
r56325Fixing User::getBlockedStatus which broke r55918. Function now works for all ...churchofemacs19:34, 14 September 2009

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r55909Display note on Special:Contributions if user is blocked, and provide an exce...churchofemacs01:29, 7 September 2009

Comments

#Comment by Nikerabbit (talk | contribs)   16:08, 7 September 2009

In function showBlock: Why $nt? Why not $title? Documentation not updated for $id. Why not $userId?

#Comment by Church of emacs (talk | contribs)   17:52, 7 September 2009

$nt and $id because contributionsSub() uses $nt and $id (I wanted to keep variable names). Changed to $title and $userId in r55982. Also added documentation for parameter $id.

#Comment by Mr.Z-man (talk | contribs)   02:16, 14 September 2009

User::getBlockedStatus() (called from isBlocked) checks for IP blocks, but seems to assume that it will only be called for the current user, so it gets the IP from wfGetIP(), which will always get the current user's IP. So if your IP is blocked, and you visit the contributions of someone with at least one entry in their block log who doesn't have ipblockexempt, it will appear as if the user is currently blocked, showing a misleading block log entry.

#Comment by Church of emacs (talk | contribs)   09:56, 14 September 2009

You're right, User::getBlockedStatus() mistakenly assumes that its checking the current user. But it is a non-static function that may get called from all kind of user objects, so this assumption is false. If we're looking at some user beside the current user, who is logged-in, we don't actually know if autoblock applies, because we don't know which IPs this user might have. I'm currently working on a fix of User::getBlockedStatus().

#Comment by Church of emacs (talk | contribs)   19:35, 14 September 2009

Hopefully fixed User::getBlockedStatus() in r56325.

Status & tagging log