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