API: Add block info to more block errors
authorBrad Jorsch <bjorsch@wikimedia.org>
Mon, 18 Feb 2019 20:30:41 +0000 (15:30 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Tue, 19 Feb 2019 22:34:48 +0000 (17:34 -0500)
When using ApiBase::errorArrayToStatus(), block info was added to
'blocked' errors. But when using dieStatus() with a Status object
returned by core MediaWiki code, block info was not being added.

Change-Id: I14887b6dd76d665055283945b956b2e26c521ed5
Depends-On: Ie3addf53ab5fabf1c24e1033b58e63927f4e21bf

RELEASE-NOTES-1.33
includes/api/ApiBase.php
tests/phpunit/includes/api/ApiBaseTest.php

index e545af8..cca2c5d 100644 (file)
@@ -96,6 +96,7 @@ production.
   deletion will be processed via the job queue.
 * action=setnotificationtimestamp will now update the watchlist asynchronously
   if entirewatchlist is set, so updates may not be visible immediately
+* Block info will be added to "blocked" errors from more modules.
 
 === Action API internal changes in 1.33 ===
 * A number of deprecated methods for API documentation, intended for overriding
index 21e20c2..1d209fd 100644 (file)
@@ -271,6 +271,14 @@ abstract class ApiBase extends ContextSource {
        /** @var int[][][] Cache for self::filterIDs() */
        private static $filterIDsCache = [];
 
+       /** $var array Map of web UI block messages to corresponding API messages and codes */
+       private static $blockMsgMap = [
+               'blockedtext' => [ 'apierror-blocked', 'blocked' ],
+               'blockedtext-partial' => [ 'apierror-blocked', 'blocked' ],
+               'autoblockedtext' => [ 'apierror-autoblocked', 'autoblocked' ],
+               'systemblockedtext' => [ 'apierror-systemblocked', 'blocked' ],
+       ];
+
        /** @var ApiMain */
        private $mMainModule;
        /** @var string */
@@ -1797,28 +1805,9 @@ abstract class ApiBase extends ContextSource {
 
                $status = Status::newGood();
                foreach ( $errors as $error ) {
-                       if ( is_array( $error ) && $error[0] === 'blockedtext' && $user->getBlock() ) {
-                               $status->fatal( ApiMessage::create(
-                                       'apierror-blocked',
-                                       'blocked',
-                                       [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $user->getBlock() ) ]
-                               ) );
-                       } elseif ( is_array( $error ) && $error[0] === 'blockedtext-partial' && $user->getBlock() ) {
-                               $status->fatal( ApiMessage::create(
-                                       'apierror-blocked-partial',
-                                       'blocked',
-                                       [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $user->getBlock() ) ]
-                               ) );
-                       } elseif ( is_array( $error ) && $error[0] === 'autoblockedtext' && $user->getBlock() ) {
-                               $status->fatal( ApiMessage::create(
-                                       'apierror-autoblocked',
-                                       'autoblocked',
-                                       [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $user->getBlock() ) ]
-                               ) );
-                       } elseif ( is_array( $error ) && $error[0] === 'systemblockedtext' && $user->getBlock() ) {
-                               $status->fatal( ApiMessage::create(
-                                       'apierror-systemblocked',
-                                       'blocked',
+                       if ( is_array( $error ) && isset( self::$blockMsgMap[$error[0]] ) && $user->getBlock() ) {
+                               list( $msg, $code ) = self::$blockMsgMap[$error[0]];
+                               $status->fatal( ApiMessage::create( $msg, $code,
                                        [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $user->getBlock() ) ]
                                ) );
                        } else {
@@ -1828,6 +1817,26 @@ abstract class ApiBase extends ContextSource {
                return $status;
        }
 
+       /**
+        * Add block info to block messages in a Status
+        * @since 1.33
+        * @param StatusValue $status
+        * @param User|null $user
+        */
+       public function addBlockInfoToStatus( StatusValue $status, User $user = null ) {
+               if ( $user === null ) {
+                       $user = $this->getUser();
+               }
+
+               foreach ( self::$blockMsgMap as $msg => list( $apiMsg, $code ) ) {
+                       if ( $status->hasMessage( $msg ) && $user->getBlock() ) {
+                               $status->replaceMessage( $msg, ApiMessage::create( $apiMsg, $code,
+                                       [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $user->getBlock() ) ]
+                               ) );
+                       }
+               }
+       }
+
        /**
         * Call wfTransactionalTimeLimit() if this request was POSTed
         * @since 1.26
@@ -2065,6 +2074,7 @@ abstract class ApiBase extends ContextSource {
                        $status = $newStatus;
                }
 
+               $this->addBlockInfoToStatus( $status );
                throw new ApiUsageException( $this, $status );
        }
 
index 8049a47..4600551 100644 (file)
@@ -1343,6 +1343,54 @@ class ApiBaseTest extends ApiTestCase {
                ], $user ) );
        }
 
+       public function testAddBlockInfoToStatus() {
+               $mock = new MockApi();
+
+               // Sanity check empty array
+               $expect = Status::newGood();
+               $test = Status::newGood();
+               $mock->addBlockInfoToStatus( $test );
+               $this->assertEquals( $expect, $test );
+
+               // No blocked $user, so no special block handling
+               $expect = Status::newGood();
+               $expect->fatal( 'blockedtext' );
+               $expect->fatal( 'autoblockedtext' );
+               $expect->fatal( 'systemblockedtext' );
+               $expect->fatal( 'mainpage' );
+               $expect->fatal( 'parentheses', 'foobar' );
+               $test = clone $expect;
+               $mock->addBlockInfoToStatus( $test );
+               $this->assertEquals( $expect, $test );
+
+               // Has a blocked $user, so special block handling
+               $user = $this->getMutableTestUser()->getUser();
+               $block = new \Block( [
+                       'address' => $user->getName(),
+                       'user' => $user->getID(),
+                       'by' => $this->getTestSysop()->getUser()->getId(),
+                       'reason' => __METHOD__,
+                       'expiry' => time() + 100500,
+               ] );
+               $block->insert();
+               $blockinfo = [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $block ) ];
+
+               $expect = Status::newGood();
+               $expect->fatal( ApiMessage::create( 'apierror-blocked', 'blocked', $blockinfo ) );
+               $expect->fatal( ApiMessage::create( 'apierror-autoblocked', 'autoblocked', $blockinfo ) );
+               $expect->fatal( ApiMessage::create( 'apierror-systemblocked', 'blocked', $blockinfo ) );
+               $expect->fatal( 'mainpage' );
+               $expect->fatal( 'parentheses', 'foobar' );
+               $test = Status::newGood();
+               $test->fatal( 'blockedtext' );
+               $test->fatal( 'autoblockedtext' );
+               $test->fatal( 'systemblockedtext' );
+               $test->fatal( 'mainpage' );
+               $test->fatal( 'parentheses', 'foobar' );
+               $mock->addBlockInfoToStatus( $test, $user );
+               $this->assertEquals( $expect, $test );
+       }
+
        public function testDieStatus() {
                $mock = new MockApi();