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
