MediaWiki r64228 - Code Review

Jump to: navigation, search
Revision:r64227‎ | r64228 (on ViewVC)‎ | r64229 >
Date:22:02, 26 March 2010
Status:ok (Comments)
(bug 15810) stop blocked admins from unblocking themselves or others.
Modified paths:

Diff [purge]

Loading diff…

Follow-up revisions

Rev.Commit summaryAuthorDate
r64230Followup to r64228 - apply restrictions in API.happy-melon23:02, 26 March 2010
r64231Follow-up r64228, per requests at bug 15810: grant admins 'unblockself' by de...happy-melon23:11, 26 March 2010
r64256Per r64228 CR: make the check a static method in IPBlockForm to reduce duplic...happy-melon15:05, 27 March 2010
r64494Better RELEASE-NOTES for r64228...simetrical18:22, 1 April 2010
r64496Better better RELEASE-NOTES for r64228...simetrical19:43, 1 April 2010
r89293'unblockself' right was never added to User::$mCoreRights...reedy21:44, 1 June 2011


#Comment by Bryan (talk | contribs)   22:31, 26 March 2010

Still circumventable via API. Permission checks should be done in IPBlockForm::doBlock itself.

#Comment by Happy-melon (talk | contribs)   23:03, 26 March 2010

Fixed in r64230.

#Comment by Bryan (talk | contribs)   10:22, 27 March 2010

Code duplication is considered bad in programming because it it doubles (or in this case quadruples) maintenance effort. There should be a static IpBlockForm::checkUnblockSelf that is called in the 4 places that you use it.

#Comment by Happy-melon (talk | contribs)   15:07, 27 March 2010

Now that makes sense. Done in r64256.

#Comment by X! (talk | contribs)   20:13, 4 May 2010

What if an admin accidentally blocks themselves? It's happened quite a bit.

#Comment by Happy-melon (talk | contribs)   20:22, 4 May 2010

They wait for someone to unblock them, just like everyone else who gets blocked accidentally. The only problem is if there're a limited number of admins on a site; especially if one goes rogue and blocks the others. Which is why it should be a configurable setting which can be on for small wikis and turned off for larger ones when that ceases to be a concern.

#Comment by FloNight (talk | contribs)   20:36, 4 May 2010

As long as it can be either turned off or on to individualize it for the needs of a wiki, I think it is a good change indeed!

#Comment by Werdna (talk | contribs)   03:52, 9 December 2010

I think this proposed feature needs more discussion, preferably in a public forum rather than a developer back channel. A change like this definitely needs community consultation.

#Comment by Bryan (talk | contribs)   07:49, 9 December 2010

Assigning the unblockself right to sysops by default would be an acceptable solution pending community consultation I think.

#Comment by Reedy (talk | contribs)   21:50, 1 June 2011

Never added to core user rights list, added in r89293

Status & tagging log

  • 15:07, 9 December 2010 Catrope (talk | contribs) changed the status of r64228 [removed: new added: ok]
  • 15:07, 27 March 2010 Happy-melon (talk | contribs) changed the status of r64228 [removed: fixme added: new]
  • 10:22, 27 March 2010 Bryan (talk | contribs) changed the status of r64228 [removed: new added: fixme]
  • 23:03, 26 March 2010 Happy-melon (talk | contribs) changed the status of r64228 [removed: fixme added: new]
  • 22:31, 26 March 2010 Bryan (talk | contribs) changed the status of r64228 [removed: new added: fixme]