* Refactoring IPBlockForm::doBlock() to return message keys

* Refactoring ApiBlock accordingly
* Adding check for blockemail right to ApiBlock
* Adding more messages to ApiBase::$messageMap
* Fixing E_NOTICE in SpecialIpblocklist.php

diff --git a/includes/SpecialBlockip.php b/includes/SpecialBlockip.php
index e93f6f6..c7ec572 100644 (file)
--- a/includes/SpecialBlockip.php
+++ b/includes/SpecialBlockip.php
@@ -282,17 +282,10 @@ class IPBlockForm {
                }
        }
 
-       const BLOCK_SUCCESS = 0; // Success
-       const BLOCK_RANGE_INVALID = 1; // Invalid IP range
-       const BLOCK_RANGE_DISABLED = 2; // Sysops can't block ranges
-       const BLOCK_NONEXISTENT_USER = 3; // No such user
-       const BLOCK_IP_INVALID = 4; // Invalid IP address
-       const BLOCK_EXPIRY_INVALID = 5; // Invalid expiry time
-       const BLOCK_ALREADY_BLOCKED = 6; // User is already blocked
        /**
         * Backend block code.
         * $userID and $expiry will be filled accordingly
-        * Returns one of the BLOCK_* constants
+        * @return array(message key, arguments) on failure, empty array on success
         */
        function doBlock(&$userId = null, &$expiry = null)
        {
@@ -313,23 +306,23 @@ class IPBlockForm {
                                # IPv4
                                if ( $wgSysopRangeBans ) {
                                        if ( !IP::isIPv4( $this->BlockAddress ) || $matches[2] < 16 || $matches[2] > 32 ) {
-                                               return self::BLOCK_RANGE_INVALID;
+                                               return array('ip_range_invalid');
                                        }
                                        $this->BlockAddress = Block::normaliseRange( $this->BlockAddress );
                                } else {
                                        # Range block illegal
-                                       return self::BLOCK_RANGE_DISABLED;
+                                       return array('range_block_disabled');
                                }
                        } else if ( preg_match( "/^($rxIP6)\\/(\\d{1,3})$/", $this->BlockAddress, $matches ) ) {
                                # IPv6
                                if ( $wgSysopRangeBans ) {
                                        if ( !IP::isIPv6( $this->BlockAddress ) || $matches[2] < 64 || $matches[2] > 128 ) {
-                                               return self::BLOCK_RANGE_INVALID;
+                                               return array('ip_range_invalid');
                                        }
                                        $this->BlockAddress = Block::normaliseRange( $this->BlockAddress );
                                } else {
                                        # Range block illegal
-                                       return self::BLOCK_RANGE_DISABLED;
+                                       return array('range_block_disabled');
                                }
                        } else {
                                # Username block
@@ -337,13 +330,12 @@ class IPBlockForm {
                                        $user = User::newFromName( $this->BlockAddress );
                                        if( !is_null( $user ) && $user->getID() ) {
                                                # Use canonical name
-                                               $this->BlockAddress = $user->getName();
                                                $userId = $user->getID();
                                        } else {
-                                               return self::BLOCK_NONEXISTENT_USER;
+                                               return array('nosuchusershort', htmlspecialchars($user->getName()));
                                        }
                                } else {
-                                       return self::BLOCK_IP_INVALID;
+                                       return array('badipaddress');
                                }
                        }
                }
@@ -361,7 +353,7 @@ class IPBlockForm {
                        $expirestr = $this->BlockOther;
 
                if (strlen($expirestr) == 0) {
-                       return self::BLOCK_EXPIRY_INVALID;
+                       return array('ipb_expiry_invalid');
                }
 
                if ( $expirestr == 'infinite' || $expirestr == 'indefinite' ) {
@@ -371,7 +363,7 @@ class IPBlockForm {
                        $expiry = strtotime( $expirestr );
 
                        if ( $expiry < 0 || $expiry === false ) {
-                               return self::BLOCK_EXPIRY_INVALID;
+                               return array('ipb_expiry_invalid');
                        }
 
                        $expiry = wfTimestamp( TS_MW, $expiry );
@@ -387,7 +379,7 @@ class IPBlockForm {
                if (wfRunHooks('BlockIp', array(&$block, &$wgUser))) {
 
                        if ( !$block->insert() ) {
-                               return self::BLOCK_ALREADY_BLOCKED;
+                               return array('ipb_already_blocked', htmlspecialchars($this->BlockAddress));
                        }
 
                        wfRunHooks('BlockIpComplete', array($block, $wgUser));
@@ -404,8 +396,10 @@ class IPBlockForm {
                          $reasonstr, $logParams );
 
                        # Report to the user
-                       return self::BLOCK_SUCCESS;
+                       return array();
                }
+               else
+                       return array('hookaborted');
        }
 
        /**
@@ -416,34 +410,14 @@ class IPBlockForm {
        {
                global $wgOut;
                $retval = $this->doBlock();
-               switch($retval)
-               {
-                       case self::BLOCK_RANGE_INVALID:
-                               $this->showForm( wfMsg( 'ip_range_invalid' ) );
-                               return;
-                       case self::BLOCK_RANGE_DISABLED:
-                               $this->showForm( wfMsg( 'range_block_disabled' ) );
-                               return;
-                       case self::BLOCK_NONEXISTENT_USER:
-                               $this->showForm( wfMsg( 'nosuchusershort', htmlspecialchars( $this->BlockAddress ) ) );
-                               return;
-                       case self::BLOCK_IP_INVALID:
-                               $this->showForm( wfMsg( 'badipaddress' ) );
-                               return;
-                       case self::BLOCK_EXPIRY_INVALID:
-                               $this->showForm( wfMsg( 'ipb_expiry_invalid' ) );
-                               return;
-                       case self::BLOCK_ALREADY_BLOCKED:
-                               $this->showForm( wfMsg( 'ipb_already_blocked', htmlspecialchars( $this->BlockAddress ) ) );
-                               return;
-                       case self::BLOCK_SUCCESS:
-                               $titleObj = SpecialPage::getTitleFor( 'Blockip' );
-                               $wgOut->redirect( $titleObj->getFullURL( 'action=success&ip=' .
-                                       urlencode( $this->BlockAddress ) ) );
-                               return;
-                       default:
-                               throw new MWException( __METHOD__ . ": Unknown return value ``{$retval}''" );
+               if(empty($retval)) {
+                       $titleObj = SpecialPage::getTitleFor( 'Blockip' );
+                       $wgOut->redirect( $titleObj->getFullURL( 'action=success&ip=' .
+                               urlencode( $this->BlockAddress ) ) );
+                       return;
                }
+               $key = array_shift($retval);
+               $this->showForm(wfMsgReal($key, $retval));
        }
 
        function showSuccess() {

diff --git a/includes/SpecialIpblocklist.php b/includes/SpecialIpblocklist.php
index 997bc5e..c46e847 100644 (file)
--- a/includes/SpecialIpblocklist.php
+++ b/includes/SpecialIpblocklist.php
@@ -200,7 +200,7 @@ class IPUnblockForm {
 
        function doSubmit() {
                global $wgOut;
-               $retval = self::doUnblock(&$this->id, &$this->ip, &$this->reason, &$range);
+               $retval = self::doUnblock($this->id, $this->ip, $this->reason, $range);
                if($retval == self::UNBLOCK_SUCCESS) {
                        # Report to the user
                        $titleObj = SpecialPage::getTitleFor( "Ipblocklist" );

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index cf5b3a6..9043815 100644 (file)
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -593,6 +593,12 @@ abstract class ApiBase {
                'cantmove-titleprotected' => array('code' => 'protectedtitle', 'info' => "The destination article has been protected from creation"),
                // 'badarticleerror' => shouldn't happen
                // 'badtitletext' => shouldn't happen
+               'ip_range_invalid' => array('code' => 'invalidrange', 'info' => "Invalid IP range"),
+               'range_block_disabled' => array('code' => 'rangedisabled', 'info' => "Blocking IP ranges has been disabled"),
+               'nosuchusershort' => array('code' => 'nosuchuser', 'info' => "The user you specified doesn't exist"),
+               'badipaddress' => array('code' => 'invalidip', 'info' => "Invalid IP address specified"),
+               'ipb_expiry_invalid' => array('code' => 'invalidexpiry', 'info' => "Invalid expiry time"),
+               'ipb_already_blocked' => array('code' => 'alreadyblocked', 'info' => "The user you tried to block was already blocked"),
                
                // API-specific messages
                'missingparam' => array('code' => 'no$1', 'info' => "The \$1 parameter must be set"),
@@ -602,6 +608,9 @@ abstract class ApiBase {
                'pastexpiry' => array('code' => 'pastexpiry', 'info' => "Expiry time is in the past"),
                'create-titleexists' => array('code' => 'create-titleexists', 'info' => "Existing titles can't be protected with 'create'"),
                'missingtitle-createonly' => array('code' => 'missingtitle-createonly', 'info' => "Missing titles can only be protected with 'create'"),
+               'cantblock' => array('code' => 'cantblock', 'info' => "You don't have permission to block users"),
+               'canthide' => array('code' => 'canthide', 'info' => "You don't have permission to hide user names from the block log"),
+               'cantblock-email' => array('code' => 'cantblock-email', 'info' => "You don't have permission to block users from sending e-mail through the wiki"),
        );
        
        /**

diff --git a/includes/api/ApiBlock.php b/includes/api/ApiBlock.php
index a91657e..f59e235 100644 (file)
--- a/includes/api/ApiBlock.php
+++ b/includes/api/ApiBlock.php
@@ -61,21 +61,23 @@ class ApiBlock extends ApiBase {
                }
 
                if(is_null($params['user']))
-                       $this->dieUsage('The user parameter must be set', 'nouser');
+                       $this->dieUsageMsg(array('missingparam', 'user'));
                if(is_null($params['token']))
-                       $this->dieUsage('The token parameter must be set', 'notoken');
+                       $this->dieUsageMsg(array('missingparam', 'token'));
                if(!$wgUser->matchEditToken($params['token']))
-                       $this->dieUsage('Invalid token', 'badtoken');
+                       $this->dieUsageMsg(array('sessionfailure'));
                if(!$wgUser->isAllowed('block'))
-                       $this->dieUsage('You don\'t have permission to block users', 'permissiondenied');
+                       $this->dieUsageMsg(array('cantblock'));
                if($params['hidename'] && !$wgUser->isAllowed('hideuser'))
-                       $this->dieUsage('You don\'t have permission to hide user names from the block log', 'nohide');
+                       $this->dieUsageMsg(array('canthide'));
+               if($params['noemail'] && !$wgUser->isAllowed('blockemail'))
+                       $this->dieUsageMsg(array('cantblock-email'));
                if(wfReadOnly())
-                       $this->dieUsage('The wiki is in read-only mode', 'readonly');
+                       $this->dieUsageMsg(array('readonlytext'));
 
                $form = new IPBlockForm('');
                $form->BlockAddress = $params['user'];
-               $form->BlockReason = $params['reason'];
+               $form->BlockReason = (is_null($params['reason']) ? '' : $params['reason']);
                $form->BlockReasonList = 'other';
                $form->BlockExpiry = ($params['expiry'] == 'never' ? 'infinite' : $params['expiry']);
                $form->BlockOther = '';
@@ -88,27 +90,11 @@ class ApiBlock extends ApiBase {
                $dbw = wfGetDb(DB_MASTER);
                $dbw->begin();
                $retval = $form->doBlock($userID, $expiry);
-               switch($retval)
-               {
-                       case IPBlockForm::BLOCK_SUCCESS:
-                               break; // We'll deal with that later
-                       case IPBlockForm::BLOCK_RANGE_INVALID:
-                               $this->dieUsage("Invalid IP range ``{$params['user']}''", 'invalidrange');
-                       case IPBlockForm::BLOCK_RANGE_DISABLED:
-                               $this->dieUsage('Blocking IP ranges has been disabled', 'rangedisabled');
-                       case IPBlockForm::BLOCK_NONEXISTENT_USER:
-                               $this->dieUsage("User ``{$params['user']}'' doesn't exist", 'nosuchuser');
-                       case IPBlockForm::BLOCK_IP_INVALID:
-                               $this->dieUsage("Invaild IP address ``{$params['user']}''", 'invalidip');
-                       case IPBlockForm::BLOCK_EXPIRY_INVALID:
-                               $this->dieUsage("Invalid expiry time ``{$params['expiry']}''", 'invalidexpiry');
-                       case IPBlockForm::BLOCK_ALREADY_BLOCKED:
-                               $this->dieUsage("User ``{$params['user']}'' is already blocked", 'alreadyblocked');
-                       default:
-                               $this->dieDebug(__METHOD__, "IPBlockForm::doBlock() returned an unknown error ($retval)");
-               }
+               if(!empty($retval))
+                       // We don't care about multiple errors, just report one of them
+                       $this->dieUsageMsg($retval);
+
                $dbw->commit();
-               
                $res['user'] = $params['user'];
                $res['userID'] = $userID;
                $res['expiry'] = ($expiry == Block::infinity() ? 'infinite' : $expiry);
@@ -152,7 +138,7 @@ class ApiBlock extends ApiBase {
                        'anononly' => 'Block anonymous users only (i.e. disable anonymous edits for this IP)',
                        'nocreate' => 'Prevent account creation',
                        'autoblock' => 'Automatically block the last used IP address, and any subsequent IP addresses they try to login from',
-                       'noemail' => 'Prevent user from sending e-mail through the wiki',
+                       'noemail' => 'Prevent user from sending e-mail through the wiki. (Requires the "blockemail" right.)',
                        'hidename' => 'Hide the username from the block log. (Requires the "hideuser" right.)'
                );
        }