Return errors from WatchAction

Currently, WatchAction::doWatch and WatchAction::doUnwatch return true
always. Let's have them return a status object instead.

This also cleans up the handling of Status objects in some of the API
modules.

Change-Id: I9dd9f0fd499c37f29fa12bcdb6142238a1f11e4d

diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index 203880e..41eaa98 100644 (file)
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
@@ -185,6 +185,7 @@ production.
   user blocks.
 * (bug 48201) action=parse&text=foo now assumes wikitext if no title is given,
   rather than using the content model of the page "API".
+* action=watch may now return errors.
 
 === Languages updated in 1.22===
 

diff --git a/docs/hooks.txt b/docs/hooks.txt
index 46949ea..e5444ce 100644 (file)
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -2425,6 +2425,7 @@ $article: article "acted on"
 'UnwatchArticle': Before a watch is removed from an article.
 $user: user watching
 $page: WikiPage object to be removed
+&$status: Status object to be returned if the hook returns false
 
 'UnwatchArticleComplete': After a watch is removed from an article.
 $user: user that watched
@@ -2674,6 +2675,7 @@ used to alter the SQL query which gets the list of wanted pages.
 'WatchArticle': Before a watch is added to an article.
 $user: user that will watch
 $page: WikiPage object to be watched
+&$status: Status object to be returned if the hook returns false
 
 'WatchArticleComplete': After a watch is added to an article.
 $user: user that watched

diff --git a/includes/actions/WatchAction.php b/includes/actions/WatchAction.php
index de9e1d6..b1c4811 100644 (file)
--- a/includes/actions/WatchAction.php
+++ b/includes/actions/WatchAction.php
@@ -87,24 +87,42 @@ class WatchAction extends FormAction {
                return parent::checkCanExecute( $user );
        }
 
+       /**
+        * Watch a page
+        * @since 1.22 Returns Status object
+        * @param Title $title Page to watch/unwatch
+        * @param User $user User who is watching/unwatching
+        * @return Status
+        */
        public static function doWatch( Title $title, User $user ) {
                $page = WikiPage::factory( $title );
 
-               if ( wfRunHooks( 'WatchArticle', array( &$user, &$page ) ) ) {
+               $status = Status::newFatal( 'hookaborted' );
+               if ( wfRunHooks( 'WatchArticle', array( &$user, &$page, &$status ) ) ) {
+                       $status = Status::newGood();
                        $user->addWatch( $title );
                        wfRunHooks( 'WatchArticleComplete', array( &$user, &$page ) );
                }
-               return true;
+               return $status;
        }
 
+       /**
+        * Unwatch a page
+        * @since 1.22 Returns Status
+        * @param Title $title Page to watch/unwatch
+        * @param User $user User who is watching/unwatching
+        * @return Status
+        */
        public static function doUnwatch( Title $title, User $user ) {
                $page = WikiPage::factory( $title );
 
-               if ( wfRunHooks( 'UnwatchArticle', array( &$user, &$page ) ) ) {
+               $status = Status::newFatal( 'hookaborted' );
+               if ( wfRunHooks( 'UnwatchArticle', array( &$user, &$page, &$status ) ) ) {
+                       $status = Status::newGood();
                        $user->removeWatch( $title );
                        wfRunHooks( 'UnwatchArticleComplete', array( &$user, &$page ) );
                }
-               return true;
+               return $status;
        }
 
        /**

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index 57287d7..e6e784f 100644 (file)
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -1221,6 +1221,44 @@ abstract class ApiBase extends ContextSource {
                throw new UsageException( $description, $this->encodeParamName( $errorCode ), $httpRespCode, $extradata );
        }
 
+       /**
+        * Throw a UsageException based on the errors in the Status object.
+        *
+        * @since 1.22
+        * @param Status $status Status object
+        * @throws UsageException
+        */
+       public function dieStatus( $status ) {
+               if ( $status->isGood() ) {
+                       throw new MWException( 'Successful status passed to ApiBase::dieStatus' );
+               }
+
+               $errors = $status->getErrorsArray();
+               if ( !$errors ) {
+                       // No errors? Assume the warnings should be treated as errors
+                       $errors = $status->getWarningsArray();
+               }
+               if ( !$errors ) {
+                       // Still no errors? Punt
+                       $errors = array( array( 'unknownerror-nocode' ) );
+               }
+
+               // Cannot use dieUsageMsg() because extensions might return custom
+               // error messages.
+               if ( $errors[0] instanceof Message ) {
+                       $msg = $errors[0];
+                       $code = $msg->getKey();
+               } else {
+                       $code = array_shift( $errors[0] );
+                       $msg = wfMessage( $code, $errors[0] );
+               }
+               if ( isset( ApiBase::$messageMap[$code] ) ) {
+                       // Translate message to code, for backwards compatability
+                       $code = ApiBase::$messageMap[$code]['code'];
+               }
+               $this->dieUsage( $msg->inLanguage( 'en' )->useDatabase( false )->plain(), $code );
+       }
+
        /**
         * Array that maps message keys to error messages. $1 and friends are replaced.
         */

diff --git a/includes/api/ApiCreateAccount.php b/includes/api/ApiCreateAccount.php
index 59ff324..9464d49 100644 (file)
--- a/includes/api/ApiCreateAccount.php
+++ b/includes/api/ApiCreateAccount.php
@@ -128,17 +128,7 @@ class ApiCreateAccount extends ApiBase {
                        $result['result'] = 'needtoken';
                } elseif ( !$status->isOK() ) {
                        // There was an error. Die now.
-                       // Cannot use dieUsageMsg() directly because extensions
-                       // might return custom error messages.
-                       $errors = $status->getErrorsArray();
-                       if ( $errors[0] instanceof Message ) {
-                               $code = 'aborted';
-                               $desc = $errors[0];
-                       } else {
-                               $code = array_shift( $errors[0] );
-                               $desc = wfMessage( $code, $errors[0] );
-                       }
-                       $this->dieUsage( $desc, $code );
+                       $this->dieStatus( $status );
                } elseif ( !$status->isGood() ) {
                        // Status is not good, but OK. This means warnings.
                        $result['result'] = 'warning';

diff --git a/includes/api/ApiDelete.php b/includes/api/ApiDelete.php
index d1f0806..aea1048 100644 (file)
--- a/includes/api/ApiDelete.php
+++ b/includes/api/ApiDelete.php
@@ -61,8 +61,7 @@ class ApiDelete extends ApiBase {
                        $this->dieUsageMsg( $status[0] );
                }
                if ( !$status->isGood() ) {
-                       $errors = $status->getErrorsArray();
-                       $this->dieUsageMsg( $errors[0] ); // We don't care about multiple errors, just report one of them
+                       $this->dieStatus( $status );
                }
 
                // Deprecated parameters

diff --git a/includes/api/ApiImport.php b/includes/api/ApiImport.php
index 56af76f..f48a822 100644 (file)
--- a/includes/api/ApiImport.php
+++ b/includes/api/ApiImport.php
@@ -57,7 +57,7 @@ class ApiImport extends ApiBase {
                        $source = ImportStreamSource::newFromUpload( 'xml' );
                }
                if ( !$source->isOK() ) {
-                       $this->dieUsageMsg( $source->getErrorsArray() );
+                       $this->dieStatus( $source );
                }
 
                $importer = new WikiImporter( $source->value );
@@ -67,7 +67,7 @@ class ApiImport extends ApiBase {
                if ( isset( $params['rootpage'] ) ) {
                        $statusRootPage = $importer->setTargetRootPage( $params['rootpage'] );
                        if ( !$statusRootPage->isGood() ) {
-                               $this->dieUsageMsg( $statusRootPage->getErrorsArray() );
+                               $this->dieStatus( $statusRootPage );
                        }
                }
                $reporter = new ApiImportReporter(

diff --git a/includes/api/ApiProtect.php b/includes/api/ApiProtect.php
index 503c692..7830c8b 100644 (file)
--- a/includes/api/ApiProtect.php
+++ b/includes/api/ApiProtect.php
@@ -103,8 +103,7 @@ class ApiProtect extends ApiBase {
                $status = $pageObj->doUpdateRestrictions( $protections, $expiryarray, $cascade, $params['reason'], $this->getUser() );
 
                if ( !$status->isOK() ) {
-                       $errors = $status->getErrorsArray();
-                       $this->dieUsageMsg( $errors[0] );
+                       $this->dieStatus( $status );
                }
                $res = array(
                        'title' => $titleObj->getPrefixedText(),

diff --git a/includes/api/ApiUserrights.php b/includes/api/ApiUserrights.php
index 870201e..7d30828 100644 (file)
--- a/includes/api/ApiUserrights.php
+++ b/includes/api/ApiUserrights.php
@@ -66,8 +66,7 @@ class ApiUserrights extends ApiBase {
                $form->setContext( $this->getContext() );
                $status = $form->fetchUser( $params['user'] );
                if ( !$status->isOK() ) {
-                       $errors = $status->getErrorsArray();
-                       $this->dieUsageMsg( $errors[0] );
+                       $this->dieStatus( $status );
                } else {
                        $user = $status->value;
                }

diff --git a/includes/api/ApiWatch.php b/includes/api/ApiWatch.php
index 3e51299..e001be3 100644 (file)
--- a/includes/api/ApiWatch.php
+++ b/includes/api/ApiWatch.php
@@ -57,19 +57,19 @@ class ApiWatch extends ApiBase {
                if ( $params['unwatch'] ) {
                        $res['unwatched'] = '';
                        $res['message'] = $this->msg( 'removedwatchtext', $title->getPrefixedText() )->title( $title )->parseAsBlock();
-                       $success = UnwatchAction::doUnwatch( $title, $user );
+                       $status = UnwatchAction::doUnwatch( $title, $user );
                } else {
                        $res['watched'] = '';
                        $res['message'] = $this->msg( 'addedwatchtext', $title->getPrefixedText() )->title( $title )->parseAsBlock();
-                       $success = WatchAction::doWatch( $title, $user );
+                       $status = WatchAction::doWatch( $title, $user );
                }
 
                if ( !is_null( $oldLang ) ) {
                        $this->getContext()->setLanguage( $oldLang ); // Reset language to $oldLang
                }
 
-               if ( !$success ) {
-                       $this->dieUsageMsg( 'hookaborted' );
+               if ( !$status->isOK() ) {
+                       $this->dieStatus( $status );
                }
                $this->getResult()->addValue( null, $this->getModuleName(), $res );
        }