UserrightsPage:

* Moving fetchUser() DB logic to fetchUser_real(), fetchUser() remains present as UI wrapper
* Introducing FETCHUSER_* error constants
* saveUserGroups() now takes a User/UserRightsProxy object rather than a username
* Also validating for groups the user already has (or doesn't have) using splitGroups()
* Fixing ignored parameter $reason in saveUserGroups()

ApiChangeRights:
* Now using UserrightsPage::fetchUser_real()
* Fixing fatal error from ApiResult::setIndexedTagName() when $params['addto'] or $params['rmfrom'] was false.

diff --git a/includes/SpecialUserrights.php b/includes/SpecialUserrights.php
index fb4af20..9cc1e61 100644 (file)
--- a/includes/SpecialUserrights.php
+++ b/includes/SpecialUserrights.php
@@ -69,7 +69,7 @@ class UserrightsPage extends SpecialPage {
                                $reason = $wgRequest->getVal( 'user-reason' );
                                if( $wgUser->matchEditToken( $wgRequest->getVal( 'wpEditToken' ), $this->mTarget ) ) {
                                        $this->saveUserGroups(
-                                               $this->mTarget,
+                                               $this->fetchUser($this->mTarget),
                                                $wgRequest->getArray( 'removable' ),
                                                $wgRequest->getArray( 'available' ),
                                                $reason
@@ -88,24 +88,23 @@ class UserrightsPage extends SpecialPage {
         * Save user groups changes in the database.
         * Data comes from the editUserGroupsForm() form function
         *
-        * @param string $username Username to apply changes to.
+        * @param object $user User or UserRightsProxy to apply changes to.
         * @param array $removegroup id of groups to be removed.
         * @param array $addgroup id of groups to be added.
         * @param string $reason Reason for group change
         * @return null
         */
-       function saveUserGroups( $username, &$removegroup, &$addgroup, $reason = '') {
-               $user = $this->fetchUser( $username );
+       function saveUserGroups( $user, &$removegroup, &$addgroup, $reason = '') {
                if( !$user ) {
                        return;
                }
-               
+
                // Validate input set...
-               $changeable = $this->changeableGroups();
+               $changeable = $this->splitGroups($user->getEffectiveGroups());
                $removegroup = array_unique(
-                       array_intersect( (array)$removegroup, $changeable['remove'] ) );
+                       array_intersect( (array)$removegroup, $changeable[1] ) );
                $addgroup = array_unique(
-                       array_intersect( (array)$addgroup, $changeable['add'] ) );
+                       array_intersect( (array)$addgroup, $changeable[0] ) );
                
                // If nothing is changed, no log entry should be created
                if($removegroup == $addgroup)
@@ -143,7 +142,7 @@ class UserrightsPage extends SpecialPage {
                global $wgRequest;
                $log->addEntry( 'rights',
                        $user->getUserPage(),
-                       $wgRequest->getText( 'user-reason' ),
+                       $reason,
                        array(
                                $this->makeGroupNameList( $oldGroups ),
                                $this->makeGroupNameList( $newGroups )
@@ -180,7 +179,44 @@ class UserrightsPage extends SpecialPage {
         * @return mixed User, UserRightsProxy, or null
         */
        function fetchUser( $username ) {
-               global $wgOut, $wgUser;
+               global $wgOut;
+               $retval = $this->fetchUser_real($username);
+               if(!is_array($retval))
+                       return $retval;
+               switch($retval[0])
+               {
+                       case self::FETCHUSER_NO_INTERWIKI:
+                               $wgOut->addWikiText( wfMsg( 'userrights-no-interwiki' ) );
+                               break;
+                       case self::FETCHUSER_NO_DATABASE:
+                               $wgOut->addWikiText( wfMsg( 'userrights-nodatabase', $retval[1] ) );
+                               break;
+                       case self::FETCHUSER_NO_USER:
+                               $wgOut->addWikiText( wfMsg( 'nouserspecified' ) );
+                               break;
+                       case self::FETCHUSER_NOSUCH_USERID:
+                               $wgOut->addWikiText( wfMsg( 'noname' ) );
+                               break;
+                       case self::FETCHUSER_NOSUCH_USERNAME:
+                               $wgOut->addWikiText( wfMsg( 'nosuchusershort', wfEscapeWikiText( $retval[1] ) ) );
+                               break;
+               }
+               return null;
+       }
+       
+       /**
+        * Backend for fetchUser()
+        *
+        * @return mixed User, UserRightsProxy, or array(error code, argument)
+        */
+       const FETCHUSER_NO_INTERWIKI = -1; // User is not allowed to change rights cross-wiki; no argument
+       const FETCHUSER_NO_DATABASE = -2; // Specified database doesn't exist or isn't local; argument=database
+       const FETCHUSER_NO_USER = -3; // No user name specified; no argument
+       const FETCHUSER_NOSUCH_USERID = -4; // Specified user ID doesn't exist; argument=userid
+       const FETCHUSER_NOSUCH_USERNAME = -5; // There is no user by this name; argument=username
+       
+       function fetchUser_real( $username ) {
+               global $wgUser;
 
                $parts = explode( '@', $username );
                if( count( $parts ) < 2 ) {
@@ -190,18 +226,15 @@ class UserrightsPage extends SpecialPage {
                        list( $name, $database ) = array_map( 'trim', $parts );
 
                        if( !$wgUser->isAllowed( 'userrights-interwiki' ) ) {
-                               $wgOut->addWikiText( wfMsg( 'userrights-no-interwiki' ) );
-                               return null;
+                               return array(self::FETCHUSER_NO_INTERWIKI, null);
                        }
                        if( !UserRightsProxy::validDatabase( $database ) ) {
-                               $wgOut->addWikiText( wfMsg( 'userrights-nodatabase', $database ) );
-                               return null;
+                               return array(self::FETCHUSER_NO_DATABASE, $database);
                        }
                }
                
                if( $name == '' ) {
-                       $wgOut->addWikiText( wfMsg( 'nouserspecified' ) );
-                       return false;
+                       return array(self::FETCHUSER_NO_USER, null);
                }
                
                if( $name{0} == '#' ) {
@@ -216,8 +249,7 @@ class UserrightsPage extends SpecialPage {
                        }
                        
                        if( !$name ) {
-                               $wgOut->addWikiText( wfMsg( 'noname' ) );
-                               return null;
+                               return array(self::FETCHUSER_NOSUCH_USERID, $id);
                        }
                }
                
@@ -228,8 +260,7 @@ class UserrightsPage extends SpecialPage {
                }
                
                if( !$user || $user->isAnon() ) {
-                       $wgOut->addWikiText( wfMsg( 'nosuchusershort', wfEscapeWikiText( $username ) ) );
-                       return null;
+                       return array(self::FETCHUSER_NOSUCH_USERNAME, $username);
                }
                
                return $user;

diff --git a/includes/api/ApiChangeRights.php b/includes/api/ApiChangeRights.php
index 6d349f8..2bfcbf1 100644 (file)
--- a/includes/api/ApiChangeRights.php
+++ b/includes/api/ApiChangeRights.php
@@ -47,22 +47,29 @@ class ApiChangeRights extends ApiBase {
                $ur = new UserrightsPage($wgRequest);\r
                $allowed = $ur->changeableGroups();\r
                $res = array();\r
-\r
-               if(is_null($params['user']))\r
-                       $this->dieUsage('The user parameter must be set', 'nouser');\r
-\r
-               $uName = User::getCanonicalName($params['user']);\r
-               $u = User::newFromName($uName);\r
-               if(!$u)\r
-                       $this->dieUsage("Invalid username ``{$params['user']}''", 'invaliduser');\r
-               if($u->getId() == 0) // Anon or non-existent\r
-                       $this->dieUsage("User ``{$params['user']}'' doesn't exist", 'nosuchuser');\r
+               \r
+               $u = $ur->fetchUser_real($params['user']);\r
+               if(is_array($u))\r
+                       switch($u[0])\r
+                       {\r
+                               case UserrightsPage::FETCHUSER_NO_INTERWIKI:\r
+                                       $this->dieUsage("You don't have permission to change users' rights on other wikis", 'nointerwiki');\r
+                               case UserrightsPage::FETCHUSER_NO_DATABASE:\r
+                                       $this->dieUsage("Database ``{$u[1]}'' does not exist or is not local", 'nosuchdatabase');\r
+                               case UserrightsPage::FETCHUSER_NO_USER:\r
+                                       $this->dieUsage("You specified an empty username, or none at all", 'emptyuser');\r
+                               case UserrightsPage::FETCHUSER_NOSUCH_USERID:\r
+                                       $this->dieUsage("There is no user with ID ``{$u[1]}''", 'nosuchuserid');\r
+                               case UserrightsPage::FETCHUSER_NOSUCH_USERNAME:\r
+                                       $this->dieUsage("There is no user with username ``{$u[1]}''", 'nosuchusername');\r
+                               default:\r
+                                       $this->dieDebug(__METHOD__, "UserrightsPage::fetchUser_real() returned an unknown error ({$u[0]})");\r
+                       }\r
 \r
                $curgroups = $u->getGroups();\r
-\r
                if($params['listgroups'])\r
                {\r
-                       $res['user'] = $uName;\r
+                       $res['user'] = $u->getName();\r
                        $res['allowedgroups'] = $allowed;\r
                        $res['ingroups'] = $curgroups;\r
                        $this->getResult()->setIndexedTagName($res['ingroups'], 'group');\r
@@ -72,7 +79,7 @@ class ApiChangeRights extends ApiBase {
 ;\r
                if($params['gettoken'])\r
                {\r
-                       $res['changerightstoken'] = $wgUser->editToken($uName);\r
+                       $res['changerightstoken'] = $wgUser->editToken($u->getName());\r
                        $this->getResult()->addValue(null, $this->getModuleName(), $res);\r
                        return;\r
                }\r
@@ -81,16 +88,16 @@ class ApiChangeRights extends ApiBase {
                        $this->dieUsage('At least one of the addto and rmfrom parameters must be set', 'noaddrm');\r
                if(is_null($params['token']))\r
                        $this->dieUsage('The token parameter must be set', 'notoken');\r
-               if(!$wgUser->matchEditToken($params['token'], $uName))\r
+               if(!$wgUser->matchEditToken($params['token'], $u->getName()))\r
                        $this->dieUsage('Invalid token', 'badtoken');\r
 \r
                $dbw = wfGetDb(DB_MASTER);\r
                $dbw->begin();\r
-               $ur->saveUserGroups($uName, $params['rmfrom'], $params['addto'], $params['reason']);\r
+               $ur->saveUserGroups($u, $params['rmfrom'], $params['addto'], $params['reason']);\r
                $dbw->commit();\r
-               $res['user'] = $uName;\r
-               $res['addedto'] = $params['addto'];\r
-               $res['removedfrom'] = $params['rmfrom'];\r
+               $res['user'] = $u->getName();\r
+               $res['addedto'] = (array)$params['addto'];\r
+               $res['removedfrom'] = (array)$params['rmfrom'];\r
                $res['reason'] = $params['reason'];\r
 \r
                $this->getResult()->setIndexedTagName($res['addedto'], 'group');\r