From ace0338421b6f100d4fde475206d165bd0b3afeb Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Mon, 18 Feb 2019 15:30:41 -0500 Subject: [PATCH] API: Add block info to more block errors 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 | 1 + includes/api/ApiBase.php | 54 +++++++++++++--------- tests/phpunit/includes/api/ApiBaseTest.php | 48 +++++++++++++++++++ 3 files changed, 81 insertions(+), 22 deletions(-) diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33 index e545af8557..cca2c5dfc3 100644 --- a/RELEASE-NOTES-1.33 +++ b/RELEASE-NOTES-1.33 @@ -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 diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index 21e20c27f9..1d209fdb09 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -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 ); } diff --git a/tests/phpunit/includes/api/ApiBaseTest.php b/tests/phpunit/includes/api/ApiBaseTest.php index 8049a47f25..4600551993 100644 --- a/tests/phpunit/includes/api/ApiBaseTest.php +++ b/tests/phpunit/includes/api/ApiBaseTest.php @@ -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(); -- 2.20.1