jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/332478 )
Change subject: ApiErrorFormatter_BackCompat: Use first error, not last ...................................................................... ApiErrorFormatter_BackCompat: Use first error, not last Before Iae0e2ce3b, the only place in the API that had to deal with choosing from multiple errors was ApiBase::dieStatus(), which chose the first one in the Status object. Iae0e2ce3b changed this to choose the last one instead, which is an unnecessary backwards compatibility break. While we could make the change in ApiBase::dieStatus(), it's cleaner to change ApiErrorFormatter_BackCompat's behavior instead since it seems unlikely anything else was using that code path. Bug: T155268 Change-Id: Ia06527f8480c3d4a689792ceb8671b0d399ffbe3 --- M includes/api/ApiErrorFormatter.php M tests/phpunit/includes/api/ApiErrorFormatterTest.php 2 files changed, 24 insertions(+), 8 deletions(-) Approvals: Legoktm: Looks good to me, approved jenkins-bot: Verified Jforrester: Looks good to me, but someone else must approve diff --git a/includes/api/ApiErrorFormatter.php b/includes/api/ApiErrorFormatter.php index 814004a..c52b731 100644 --- a/includes/api/ApiErrorFormatter.php +++ b/includes/api/ApiErrorFormatter.php @@ -414,12 +414,17 @@ if ( $tag === 'error' ) { // In BC mode, only one error - $value = [ - 'code' => $msg->getApiCode(), - 'info' => $value, - ] + $msg->getApiData(); - $this->result->addValue( null, 'error', $value, - ApiResult::OVERRIDE | ApiResult::ADD_ON_TOP | ApiResult::NO_SIZE_CHECK ); + $existingError = $this->result->getResultData( [ 'error' ] ); + if ( !is_array( $existingError ) || + !isset( $existingError['code'] ) || !isset( $existingError['info'] ) + ) { + $value = [ + 'code' => $msg->getApiCode(), + 'info' => $value, + ] + $msg->getApiData(); + $this->result->addValue( null, 'error', $value, + ApiResult::OVERRIDE | ApiResult::ADD_ON_TOP | ApiResult::NO_SIZE_CHECK ); + } } else { if ( $modulePath === null ) { $moduleName = 'unknown'; diff --git a/tests/phpunit/includes/api/ApiErrorFormatterTest.php b/tests/phpunit/includes/api/ApiErrorFormatterTest.php index a40db24..eaa4d17 100644 --- a/tests/phpunit/includes/api/ApiErrorFormatterTest.php +++ b/tests/phpunit/includes/api/ApiErrorFormatterTest.php @@ -439,8 +439,8 @@ $formatter->addMessagesFromStatus( 'status', $status ); $this->assertSame( [ 'error' => [ - 'code' => 'parentheses', - 'info' => $parensPlain, + 'code' => 'mainpage', + 'info' => $mainpagePlain, ], 'warnings' => [ 'status' => [ @@ -502,6 +502,17 @@ $formatter->arrayFromStatus( $status, 'warning' ), 'arrayFromStatus test for warning' ); + + $result->reset(); + $result->addValue( null, 'error', [ 'bogus' ] ); + $formatter->addError( 'err', 'mainpage' ); + $this->assertSame( [ + 'error' => [ + 'code' => 'mainpage', + 'info' => $mainpagePlain, + ], + ApiResult::META_TYPE => 'assoc', + ], $result->getResultData(), 'Overwrites bogus "error" value with real error' ); } /** -- To view, visit https://gerrit.wikimedia.org/r/332478 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia06527f8480c3d4a689792ceb8671b0d399ffbe3 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Anomie <bjor...@wikimedia.org> Gerrit-Reviewer: Gergő Tisza <gti...@wikimedia.org> Gerrit-Reviewer: Jforrester <jforres...@wikimedia.org> Gerrit-Reviewer: Legoktm <lego...@member.fsf.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits