From d47835029553e2f2f0be7448bb8856fcbf6411d2 Mon Sep 17 00:00:00 2001 From: Roan Kattouw Date: Tue, 27 May 2008 14:42:51 +0000 Subject: [PATCH] * Refactor Title::isValidMoveOperation() and Title::moveTo() to return an array of arrays like Title::getUserPermissionsErrors() does; other functions used by the write API have undergone similar refactoring earlier * Handle these return values in MovePageForm::doSubmit() and ApiMove::execute() * Remove separate AbortMove hook calls from MovePageForm and ApiMove; these were used to capture the hook error, but the new return type handles that. Also, it resulted in two calls to that hook for each move * Remove comment about SpecialMovepageAfterMove hook from ApiMove::execute(): we don't need it, there's the TitleMoveComplete hook for that. SpecialMovepageAfterMove is a UI hook that doesn't belong in the API * Add imagenocrossnamespace and imagetypemismatch errors to ApiBase::$messageMap --- includes/SpecialMovepage.php | 8 +------- includes/Title.php | 31 +++++++++++++++---------------- includes/api/ApiBase.php | 2 ++ includes/api/ApiMove.php | 8 +------- 4 files changed, 19 insertions(+), 30 deletions(-) diff --git a/includes/SpecialMovepage.php b/includes/SpecialMovepage.php index 3e14a20f8f..6cccc6e763 100644 --- a/includes/SpecialMovepage.php +++ b/includes/SpecialMovepage.php @@ -271,15 +271,9 @@ class MovePageForm { return; } - $hookErr = null; - if( !wfRunHooks( 'AbortMove', array( $ot, $nt, $wgUser, &$hookErr ) ) ) { - $this->showForm( 'hookaborted', $hookErr ); - return; - } - $error = $ot->moveTo( $nt, true, $this->reason ); if ( $error !== true ) { - $this->showForm( $error ); + call_user_func_array(array($this, 'showForm'), $error[0]); return; } diff --git a/includes/Title.php b/includes/Title.php index dfd4b95685..42bfbc7ad0 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -2386,34 +2386,33 @@ class Title { /** * Check whether a given move operation would be valid. - * Returns true if ok, or a message key string for an error message - * if invalid. (Scarrrrry ugly interface this.) + * Returns true if ok, or a getUserPermissionsErrors()-like array otherwise * @param Title &$nt the new title * @param bool $auth indicates whether $wgUser's permissions * should be checked - * @return mixed true on success, message name on failure + * @return mixed True on success, getUserPermissionsErrors()-like array on failure */ public function isValidMoveOperation( &$nt, $auth = true ) { if( !$this or !$nt ) { - return 'badtitletext'; + return array(array('badtitletext')); } if( $this->equals( $nt ) ) { - return 'selfmove'; + return array(array('selfmove')); } if( !$this->isMovable() || !$nt->isMovable() ) { - return 'immobile_namespace'; + return array(array('immobile_namespace')); } $oldid = $this->getArticleID(); $newid = $nt->getArticleID(); if ( strlen( $nt->getDBkey() ) < 1 ) { - return 'articleexists'; + return array(array('articleexists')); } if ( ( '' == $this->getDBkey() ) || ( !$oldid ) || ( '' == $nt->getDBkey() ) ) { - return 'badarticleerror'; + return array(array('badarticleerror')); } // Image-specific checks @@ -2421,10 +2420,10 @@ class Title { $file = wfLocalFile( $this ); if( $file->exists() ) { if( $nt->getNamespace() != NS_IMAGE ) { - return 'imagenocrossnamespace'; + return array(array('imagenocrossnamespace')); } if( !File::checkExtensionCompatibility( $file, $nt->getDbKey() ) ) { - return 'imagetypemismatch'; + return array(array('imagetypemismatch')); } } } @@ -2436,13 +2435,13 @@ class Title { $nt->getUserPermissionsErrors('move', $wgUser), $nt->getUserPermissionsErrors('edit', $wgUser)); if($errors !== array()) - return $errors[0][0]; + return $errors; } global $wgUser; $err = null; if( !wfRunHooks( 'AbortMove', array( $this, $nt, $wgUser, &$err ) ) ) { - return 'hookaborted'; + return array(array('hookaborted', $err)); } # The move is allowed only if (1) the target doesn't exist, or @@ -2451,13 +2450,13 @@ class Title { if ( 0 != $newid ) { # Target exists; check for validity if ( ! $this->isValidMoveTarget( $nt ) ) { - return 'articleexists'; + return array(array('articleexists')); } } else { $tp = $nt->getTitleProtection(); $right = ( $tp['pt_create_perm'] == 'sysop' ) ? 'protect' : $tp['pt_create_perm']; if ( $tp and !$wgUser->isAllowed( $right ) ) { - return 'cantmove-titleprotected'; + return array(array('cantmove-titleprotected')); } } return true; @@ -2471,11 +2470,11 @@ class Title { * @param string $reason The reason for the move * @param bool $createRedirect Whether to create a redirect from the old title to the new title. * Ignored if the user doesn't have the suppressredirect right. - * @return mixed true on success, message name on failure + * @return mixed true on success, getUserPermissionsErrors()-like array on failure */ public function moveTo( &$nt, $auth = true, $reason = '', $createRedirect = true ) { $err = $this->isValidMoveOperation( $nt, $auth ); - if( is_string( $err ) ) { + if( is_array($err) ) { return $err; } diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index f5b51b0659..cd4fa1aafc 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -620,6 +620,8 @@ abstract class ApiBase { 'protectedpage' => array('code' => 'protectedpage', 'info' => "You don't have permission to perform this move"), 'hookaborted' => array('code' => 'hookaborted', 'info' => "The modification you tried to make was aborted by an extension hook"), 'cantmove-titleprotected' => array('code' => 'protectedtitle', 'info' => "The destination article has been protected from creation"), + 'imagenocrossnamespace' => array('code' => 'nonfilenamespace', 'info' => "Can't move a file to a non-file namespace"), + 'imagetypemismatch' => array('code' => 'filetypemismatch', 'info' => "The new file extension doesn't match its type"), // 'badarticleerror' => shouldn't happen // 'badtitletext' => shouldn't happen 'ip_range_invalid' => array('code' => 'invalidrange', 'info' => "Invalid IP range"), diff --git a/includes/api/ApiMove.php b/includes/api/ApiMove.php index 818f205c41..9baf3dd4a7 100644 --- a/includes/api/ApiMove.php +++ b/includes/api/ApiMove.php @@ -80,12 +80,10 @@ class ApiMove extends ApiBase { $this->dieUsageMsg(current($errors)); $hookErr = null; - if( !wfRunHooks( 'AbortMove', array( $fromTitle, $toTitle, $wgUser, &$hookErr ) ) ) - $this->dieUsageMsg(array('hookaborted', $hookErr)); $retval = $fromTitle->moveTo($toTitle, true, $params['reason'], !$params['noredirect']); if($retval !== true) - $this->dieUsageMsg(array($retval)); + $this->dieUsageMsg(reset($retval)); $r = array('from' => $fromTitle->getPrefixedText(), 'to' => $toTitle->getPrefixedText(), 'reason' => $params['reason']); if(!$params['noredirect'] || !$wgUser->isAllowed('suppressredirect')) @@ -121,10 +119,6 @@ class ApiMove extends ApiBase { $wgUser->removeWatch($toTitle); } $this->getResult()->addValue(null, $this->getModuleName(), $r); - - // This one is a problem as we don't have a special page, but some - // extensions may want to do something when the hook has succeeded. - //wfRunHooks( 'SpecialMovepageAfterMove', array( &$this , &$ot , &$nt ) ) ; } public function mustBePosted() { return true; } -- 2.20.1