Ejegg has uploaded a new change for review.
https://gerrit.wikimedia.org/r/284626
Change subject: WIP pass errors in to getForm()
......................................................................
WIP pass errors in to getForm()
Might be cool to pass all the adapter data into getForm so the
rendering classes don't need to ask the adapter anything.
Change-Id: Ie30559d8249dd17fc9c1af8fbe9898533ccc54ac
---
M gateway_common/GatewayPage.php
M gateway_forms/Form.php
M gateway_forms/Mustache.php
M gateway_forms/RapidHtml.php
M tests/MustacheFormTest.php
5 files changed, 52 insertions(+), 48 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface
refs/changes/26/284626/1
diff --git a/gateway_common/GatewayPage.php b/gateway_common/GatewayPage.php
index 8461312..a604399 100644
--- a/gateway_common/GatewayPage.php
+++ b/gateway_common/GatewayPage.php
@@ -153,15 +153,19 @@
/**
* Build and display form to user
+ * @param array|null $errors from validation or from payment result
*/
- public function displayForm() {
+ public function displayForm( $errors = null ) {
+ if ( $errors === null ) {
+ $errors = $this->adapter->getAllErrors();
+ }
$output = $this->getOutput();
$form_class = $this->getFormClass();
// TODO: use interface. static ctor.
if ( $form_class && class_exists( $form_class ) ){
$form_obj = new $form_class( $this->adapter );
- $form = $form_obj->getForm();
+ $form = $form_obj->getForm( $errors );
$output->addModules( $form_obj->getResources() );
$output->addHTML( $form );
} else {
@@ -288,10 +292,10 @@
if ( $this->isProcessImmediate() ) {
// Check form for errors
// FIXME: Should this be rolled into
adapter.doPayment?
- $form_errors = $this->validateForm();
+ $hasErrors = $this->validateForm();
// If there were errors, redisplay form,
otherwise proceed to next step
- if ( $form_errors ) {
+ if ( $hasErrors ) {
$this->displayForm();
} else {
// Attempt to process the payment, and
render the response.
@@ -303,8 +307,7 @@
}
} else { //token mismatch
$error['general']['token-mismatch'] = $this->msg(
'donate_interface-token-mismatch' );
- $this->adapter->addManualError( $error );
- $this->displayForm();
+ $this->displayForm( $error );
}
}
@@ -449,9 +452,9 @@
) );
$this->displayForm();
} elseif ( $errors = $result->getErrors() ) {
- // FIXME: Creepy. Currently, the form inspects adapter
errors. Use
- // the stuff encapsulated in PaymentResult instead.
- foreach (
$this->adapter->getTransactionResponse()->getErrors() as $code =>
$transactionError ) {
+ // Mash the transaction errors into a form-friendly
array
+ $formErrors = array();
+ foreach ( $result->getErrors() as $code =>
$transactionError ) {
$message = $transactionError['message'];
$error = array();
if ( !empty( $transactionError['context'] ) ) {
@@ -462,9 +465,9 @@
else {
$error['general'][ $code ] = $message;
}
- $this->adapter->addManualError( $error );
+ $formErrors[] = $error;
}
- $this->displayForm();
+ $this->displayForm( $formErrors );
} else {
// Success.
$thankYouPage = ResultPages::getThankYouPage(
$this->adapter );
diff --git a/gateway_forms/Form.php b/gateway_forms/Form.php
index 2c3a8d8..59b32b6 100644
--- a/gateway_forms/Form.php
+++ b/gateway_forms/Form.php
@@ -1,11 +1,6 @@
<?php
abstract class Gateway_Form {
- /**
- * An array of form errors, passed from the gateway form object
- * @var array
- */
- public $form_errors;
/**
* Gateway-specific logger
@@ -30,24 +25,16 @@
* this method allows for flexible form generation inside of child
classes
* while also providing a unified method for returning the full HTML for
* a form.
+ * @param array $errors a key-value array of validation and other errors
* @return string The entire form HTML
*/
- abstract function getForm();
+ abstract function getForm( $errors = array() );
public function __construct( $gateway ) {
$this->gateway = $gateway;
$this->scriptPath =
$this->gateway->getContext()->getConfig()->get( 'ScriptPath' );
$this->logger = DonationLoggerFactory::getLogger( $gateway );
- $gateway_errors = $this->gateway->getAllErrors();
-
- // @codeCoverageIgnoreStart
- if ( !is_array( $gateway_errors ) ){
- $gateway_errors = array();
- }
- // @codeCoverageIgnoreEnd
-
- $this->form_errors = array_merge(
DataValidator::getEmptyErrorArray(), $gateway_errors );
}
/**
diff --git a/gateway_forms/Mustache.php b/gateway_forms/Mustache.php
index d3855e8..ae9849c 100644
--- a/gateway_forms/Mustache.php
+++ b/gateway_forms/Mustache.php
@@ -28,12 +28,13 @@
/**
* Return the rendered HTML form, using template parameters from the
gateway object
*
+ * @param array $errors
* @return string
* @throw RuntimeException
*/
- public function getForm() {
+ public function getForm( $errors = array() ) {
$data = $this->getData();
- $data = $data + $this->getErrors();
+ $data = $data + $this->formatErrors( $errors );
$data = $data + $this->getUrls();
self::$country = $data['country'];
@@ -166,7 +167,7 @@
}
}
- protected function getErrors() {
+ protected function formatErrors( $errors ) {
$errors = $this->gateway->getAllErrors();
$return = array();
$return['errors'] = array();
diff --git a/gateway_forms/RapidHtml.php b/gateway_forms/RapidHtml.php
index 4fdebaf..f8cf7c7 100644
--- a/gateway_forms/RapidHtml.php
+++ b/gateway_forms/RapidHtml.php
@@ -1,6 +1,11 @@
<?php
class Gateway_Form_RapidHtml extends Gateway_Form {
+ /**
+ * An array of form errors, passed from the gateway form object
+ * @var array
+ */
+ public $form_errors;
/**
* Full path of HTML form to load
@@ -116,17 +121,10 @@
public function __construct( &$gateway ) {
global $wgDonationInterfaceHtmlFormDir;
parent::__construct( $gateway );
- $form_errors = $this->form_errors;
$this->loadValidateJs();
$ffname = $this->gateway->getData_Unstaged_Escaped( 'ffname' );
- // Get error passed via query string
- $error = $this->gateway->getRequest()->getText( 'error' );
- if ( $error ) {
- // We escape HTML here since only quotes are escaped
later
- $form_errors['general'][] = htmlspecialchars( $error );
- }
// only keep looking if we still haven't found a form that works
if ( empty( $this->html_file_path ) ){
@@ -139,28 +137,43 @@
}
}
- // fix general form error messages so it's not an array of msgs
- if ( is_array( $form_errors['general'] ) && count(
$form_errors['general'] ) ) {
- $general_errors = "";
- foreach ( $form_errors['general'] as $general_error ) {
- $general_errors .= "$general_error<br />";
- }
-
- $form_errors['general'] = $general_errors;
- }
-
$this->html_default_base_dir = $wgDonationInterfaceHtmlFormDir;
}
/**
* Return the HTML form with data added
*/
- public function getForm() {
+ public function getForm( $errors = array() ) {
+ $this->setFormErrors( $errors );
$html = $this->load_html();
$html = $this->replace_blocks( $html );
return $this->add_data( $html );
}
+ protected function setFormErrors( $errors ) {
+ $this->form_errors = array_merge(
DataValidator::getEmptyErrorArray(), $errors );
+
+ // Get error passed via query string
+ $qsError = $this->gateway->getRequest()->getText( 'error' );
+ if ( $qsError ) {
+ // We escape HTML here since only quotes are escaped
later
+ $this->form_errors['general'][] = htmlspecialchars(
$qsError );
+ }
+
+ // fix general form error messages so it's not an array of msgs
+ if (
+ is_array( $this->form_errors['general'] )
+ && count( $this->form_errors['general'] )
+ ) {
+ $general_errors = "";
+ foreach ( $this->form_errors['general'] as
$general_error ) {
+ $general_errors .= "$general_error<br />";
+ }
+
+ $this->form_errors['general'] = $general_errors;
+ }
+ }
+
/**
* Load the HTML form from a file into a string
* @return string
diff --git a/tests/MustacheFormTest.php b/tests/MustacheFormTest.php
index 85f6415..b849b2b 100644
--- a/tests/MustacheFormTest.php
+++ b/tests/MustacheFormTest.php
@@ -165,7 +165,7 @@
$this->setLanguage( $language );
$this->adapter->addRequestData( array( 'language' => $language
) );
$this->form = new Gateway_Form_Mustache( $this->adapter );
- $html = $this->form->getForm();
+ $html = $this->form->getForm( $this->adapter->getAllErrors() );
$expected = htmlspecialchars(
wfMessage( 'donate_interface-bigamount-error', '12.00',
'EUR', 'donor-supp...@worthyfoundation.org' )->text()
);
--
To view, visit https://gerrit.wikimedia.org/r/284626
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie30559d8249dd17fc9c1af8fbe9898533ccc54ac
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <eeggles...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
