Matthias Mullie has uploaded a new change for review. https://gerrit.wikimedia.org/r/143284
Change subject: Streamline API error output ...................................................................... Streamline API error output API already has a default way of failing (e.g. when passing invalid arguments). We shouldn't be outputting flow-specific errors differently: this only makes it harder to process errors, since we then have to deal with 2 different formats we have to interpret. Change-Id: I77a8cf1ae1bb73f211e982da209ae554393a4471 --- M i18n/en.json M i18n/qqq.json M includes/api/ApiFlowBaseGet.php M includes/api/ApiFlowBasePost.php 4 files changed, 71 insertions(+), 52 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow refs/changes/84/143284/1 diff --git a/i18n/en.json b/i18n/en.json index 1fb3fe0..1f94124 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -134,6 +134,7 @@ "flow-error-process-data": "An error has occurred while processing the data in your request.", "flow-error-process-wikitext": "An error has occurred while processing HTML/wikitext conversion.", "flow-error-no-index": "Failed to find an index to perform data search.", + "flow-error-no-render": "No block was able to render the request.", "flow-edit-header-placeholder": "Describe this Flow board", "flow-edit-header-submit": "Save header", "flow-edit-header-submit-overwrite": "Overwrite header", diff --git a/i18n/qqq.json b/i18n/qqq.json index 8ea0a91..65181e1 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -137,6 +137,7 @@ "flow-error-process-data": "General error message when failing to process data.", "flow-error-process-wikitext": "Error message when failing to process html/wikitext conversion.", "flow-error-no-index": "Error message when failing to find an index to perform data search.", + "flow-error-no-render": "Error message when nothing was able to render the request.", "flow-edit-header-placeholder": "Used as placeholder when editing the header of a Flow board", "flow-edit-header-submit": "Used as label for the Submit button.", "flow-edit-header-submit-overwrite": "Used as label for the Submit button, when submitting will overwrite a more recent change.", diff --git a/includes/api/ApiFlowBaseGet.php b/includes/api/ApiFlowBaseGet.php index e157092..c2c3166 100644 --- a/includes/api/ApiFlowBaseGet.php +++ b/includes/api/ApiFlowBaseGet.php @@ -35,12 +35,11 @@ // if nothing could render, we'll consider that an error (at least some // block should've been able to render a GET request) if ( !$output[$action]['result'] ) { - $output[$action] = array( - 'status' => 'error', - 'result' => array( - 'message' => 'No block was able to render the request', - 'extra' => array(), - ) + $this->getResult()->dieUsage( + wfMessage( 'flow-error-no-render' )->parse(), + 'no-render', + 200, + array() ); } else { $blocks = array_keys($output[$action]['result']); diff --git a/includes/api/ApiFlowBasePost.php b/includes/api/ApiFlowBasePost.php index 7f954e3..4887920 100644 --- a/includes/api/ApiFlowBasePost.php +++ b/includes/api/ApiFlowBasePost.php @@ -26,60 +26,78 @@ $request = $this->getModifiedRequest(); $blocksToCommit = $loader->handleSubmit( $action, $blocks, $user, $request ); - if ( count( $blocksToCommit ) ) { - $loader->commit( $workflow, $blocksToCommit ); - $savedBlocks = array(); - $result->setIndexedTagName( $savedBlocks, 'block' ); - foreach( $blocksToCommit as $block ) { - $savedBlocks[] = $block->getName(); + // If none of the blocks could process this submission, fail + if ( !count( $blocksToCommit ) ) { + $this->processError( $blocks ); + return; + } + + $loader->commit( $workflow, $blocksToCommit ); + $savedBlocks = array(); + $result->setIndexedTagName( $savedBlocks, 'block' ); + + foreach( $blocksToCommit as $block ) { + $savedBlocks[] = $block->getName(); + } + + $output[$action] = array( + 'result' => array(), + 'status' => 'ok', + 'workflow' => $workflow->isNew() ? '' : $workflow->getId()->getAlphadecimal(), + ); + + $parameters = $loader->extractBlockParameters( $request, $blocksToCommit ); + foreach( $blocksToCommit as $block ) { + // Always return parsed text to client after successful submission? + // @Todo - hacky, maybe have contentformat in the request to overwrite + // requiredWikitext + $block->unsetRequiresWikitext( $action ); + $output[$action]['result'][$block->getName()] = $block->renderAPI( Container::get( 'templating' ), $parameters[$block->getName()] ); + } + + // required until php5.4 which has the JsonSerializable interface + array_walk_recursive( $output, function( &$value ) { + if ( $value instanceof Anchor ) { + $value = $value->toArray(); + } elseif ( $value instanceof Message ) { + $value = $value->text(); } + } ); - $output[$action] = array( - 'result' => array(), - 'status' => 'ok', - 'workflow' => $workflow->isNew() ? '' : $workflow->getId()->getAlphadecimal(), - ); + $this->getResult()->addValue( null, $this->apiFlow->getModuleName(), $output ); + } - $parameters = $loader->extractBlockParameters( $request, $blocksToCommit ); - foreach( $blocksToCommit as $block ) { - // Always return parsed text to client after successful submission? - // @Todo - hacky, maybe have contentformat in the request to overwrite - // requiredWikitext - $block->unsetRequiresWikitext( $action ); - $output[$action]['result'][$block->getName()] = $block->renderAPI( Container::get( 'templating' ), $parameters[$block->getName()] ); - } + /** + * Kill the request if errors were encountered. + * Only the first error will be output: + * * dieUsage only outputs one error - we could add more as $extraData, but + * that would mean we'd have to check for flow-specific errors differently + * * most of our code just quits on the first error that's encountered, so + * outputting all encountered errors might still not cover everything + * that's wrong with the request + * + * @param array $blocks + */ + protected function processError( $blocks ) { + foreach( $blocks as $block ) { + if ( $block->hasErrors() ) { + $errors = $block->getErrors(); - // required until php5.4 which has the JsonSerializable interface - array_walk_recursive( $output, function( &$value ) { - if ( $value instanceof Anchor ) { - $value = $value->toArray(); - } elseif ( $value instanceof Message ) { - $value = $value->text(); - } - } ); - } else { - $output[$action] = array( - 'status' => 'error', - 'result' => array(), - ); - - foreach( $blocks as $block ) { - if ( $block->hasErrors() ) { - $errors = $block->getErrors(); - $nativeErrors = array(); - - foreach( $errors as $key ) { - $nativeErrors[$key]['message'] = $block->getErrorMessage( $key )->parse(); - $nativeErrors[$key]['extra'] = $block->getErrorExtra( $key ); - } - - $output[$action]['result'][$block->getName()] = $nativeErrors; + foreach( $errors as $key ) { + $this->getResult()->dieUsage( + $block->getErrorMessage( $key )->parse(), + $key, + 200, + // additional info for this message (e.g. to be used to + // enable recovery from error, like returning the most + // recent revision ID to re-submit content in the case + // of edit conflict) + array( $key => $block->getErrorExtra( $key ) ) + ); } } } - - $this->getResult()->addValue( null, $this->apiFlow->getModuleName(), $output ); } public function mustBePosted() { -- To view, visit https://gerrit.wikimedia.org/r/143284 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I77a8cf1ae1bb73f211e982da209ae554393a4471 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Flow Gerrit-Branch: master Gerrit-Owner: Matthias Mullie <mmul...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits