r101956 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r101955‎ | r101956 | r101957 >
Date:05:18, 4 November 2011
Author:jpostlethwaite
Status:ok (Comments)
Tags:fundraising 
Comment:
Added $error_map, getErrorMap(), getErrorMapByCodeAndTranslate(), getTransactionErrors(). Modified getTransactionDataLanguage() to pull language from multiple places.
Modified paths:
  • /trunk/extensions/DonationInterface/gateway_common/gateway.adapter.php (modified) (history)

Diff [purge]

Index: trunk/extensions/DonationInterface/gateway_common/gateway.adapter.php
@@ -101,9 +101,24 @@
102102 */
103103 abstract class GatewayAdapter implements GatewayType {
104104
105 - //Contains the map of THEIR var names, to OURS.
106 - //I'd have gone the other way, but we'd run into 1:many pretty quick.
107 - protected $var_map;
 105+ /**
 106+ * $error_map maps gateway errors to client errors
 107+ *
 108+ * The index of each error should map to a translation:
 109+ *
 110+ * 0 => globalcollect_gateway-response-default
 111+ *
 112+ * @var array $error_map
 113+ */
 114+ protected $error_map = array();
 115+
 116+ /**
 117+ * $var_map maps gateway variables to client variables
 118+ *
 119+ * @var array $var_map
 120+ */
 121+ protected $var_map = array();
 122+
108123 protected $accountInfo;
109124 protected $url;
110125 protected $transactions;
@@ -182,6 +197,7 @@
183198
184199 $this->setPostDefaults( $postDefaults );
185200 $this->defineTransactions();
 201+ $this->defineErrorMap();
186202 $this->defineVarMap();
187203 $this->defineAccountInfo();
188204 $this->defineReturnValueMap();
@@ -377,6 +393,74 @@
378394 }
379395
380396 /**
 397+ * getErrorMap
 398+ *
 399+ * This will also return an error message if a $code is passed.
 400+ *
 401+ * If the error code does not exist, the default message will be returned.
 402+ *
 403+ * A default message should always exist with an index of 0.
 404+ *
 405+ * NOTE: This method will check to see if the message exists in translation
 406+ * and use that message instead of the default. This would override error_map.
 407+ *
 408+ * @param string $code The error code to look up in the map
 409+ *
 410+ * @return array|string Returns @see GatewayAdapter::$error_map
 411+ */
 412+ public function getErrorMap( $code = null, $options = array() ) {
 413+
 414+ if ( isset( $options['code'] ) ) {
 415+ unset( $options['code'] );
 416+ }
 417+
 418+ extract( $options );
 419+
 420+ global $messages;
 421+
 422+ if ( is_null( $code ) ) {
 423+ return $this->error_map;
 424+ }
 425+
 426+ $translate = isset( $translate ) ? (boolean) $translate : false ;
 427+
 428+ $response_message = $this->getIdentifier() . '_gateway-response-' . $code;
 429+
 430+ $translatedMessage = wfMsg( $response_message );
 431+
 432+ // Check to see if an error message exists in translation
 433+ if ( substr( $translatedMessage, 0, 3 ) !== '<' ) {
 434+
 435+ // Message does not exist
 436+ $translatedMessage = '';
 437+ }
 438+
 439+ // If the $code does not exist, use the default code: 0
 440+ $code = !isset( $this->error_map[ $code ] ) ? 0 : $code;
 441+
 442+ $translatedMessage = ( $translate && empty( $translatedMessage ) ) ? wfMsg( $this->error_map[ $code ] ) : $translatedMessage;
 443+
 444+ // Check to see if we return the translated message.
 445+ $message = ( $translate ) ? $translatedMessage : $this->error_map[ $code ];
 446+
 447+ return $message;
 448+ }
 449+
 450+ /**
 451+ * getErrorMapByCodeAndTranslate
 452+ *
 453+ * This will take an error code and translate the message.
 454+ *
 455+ * @param string $code The error code to look up in the map
 456+ *
 457+ * @return string Returns the translated message from @see GatewayAdapter::$error_map
 458+ */
 459+ public function getErrorMapByCodeAndTranslate( $code ) {
 460+
 461+ return $this->getErrorMap( $code, array( 'translate' => true, ) );
 462+ }
 463+
 464+ /**
381465 * This function is used exclusively by the two functions that build
382466 * requests to be sent directly to external payment gateway servers. Those
383467 * two functions are buildRequestNameValueString, and (perhaps less
@@ -1526,16 +1610,54 @@
15271611 /**
15281612 * Returns false if language does not exist or string if it does exist
15291613 *
 1614+ * @todo
 1615+ * - Can we just get the language from somewhere else to make this simpler?
 1616+ *
15301617 * @return false|string
15311618 */
15321619 public function getTransactionDataLanguage() {
15331620
15341621 $data = $this->getTransactionData();
15351622
1536 - if ( is_array( $data ) && array_key_exists( 'language', $data ) ) {
1537 - return $data['language'];
 1623+ $return = false;
 1624+
 1625+ // Check for language in $data
 1626+ if ( is_array( $data ) ) {
 1627+ if ( array_key_exists( 'language', $data ) ) {
 1628+ $return = $data['language'];
 1629+ }
 1630+ }
 1631+
 1632+ // Check for language in $data['ORDER']['LANGUAGECODE']
 1633+ if ( empty( $return ) && array_key_exists( 'ORDER', $data ) ) {
 1634+
 1635+ if ( array_key_exists( 'LANGUAGECODE', $data['ORDER'] ) ) {
 1636+ $return = $data['ORDER']['LANGUAGECODE'];
 1637+ }
 1638+ }
 1639+
 1640+ // Check for language in $data['PAYMENT']['LANGUAGECODE']
 1641+ if ( empty( $return ) && array_key_exists( 'PAYMENT', $data ) ) {
 1642+
 1643+ if ( array_key_exists( 'LANGUAGECODE', $data['PAYMENT'] ) ) {
 1644+ $return = $data['PAYMENT']['LANGUAGECODE'];
 1645+ }
 1646+ }
 1647+
 1648+ return $return;
 1649+ }
 1650+
 1651+ /**
 1652+ * Returns an array of errors. This should be an empty array on success.
 1653+ *
 1654+ * @return array
 1655+ */
 1656+ public function getTransactionErrors() {
 1657+
 1658+ if ( is_array( $this->transaction_results ) && array_key_exists( 'errors', $this->transaction_results ) ) {
 1659+ return $this->transaction_results['errors'];
15381660 } else {
1539 - return false;
 1661+ return array();
15401662 }
15411663 }
15421664

Follow-up revisions

RevisionCommit summaryAuthorDate
r101957Added defineErrorMap().jpostlethwaite05:19, 4 November 2011
r101958Added getGoToThankYouOn(). This is a lookup to see which statuses are accepte...jpostlethwaite05:30, 4 November 2011
r101959Added defineErrorMap(), Added new code range. Added defineGoToThankYouOn(). U...jpostlethwaite05:36, 4 November 2011
r101960Removing usage of $result and $data from the view. Enabling bank transfer, re...jpostlethwaite05:41, 4 November 2011
r102236MFT r90286, r100671, r100837, r100950, r101060, r101063, r101064, r101073, r1......khorn03:06, 7 November 2011
r102237MFT r90286, r100671, r100837, r100950, r101060, r101063, r101064, r101073, r1......khorn03:07, 7 November 2011
r102469Simplfying method getTransactionDataLanguage(). See r101956.jpostlethwaite00:00, 9 November 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r101955Added resultHandler() and resultHandlerError().jpostlethwaite05:14, 4 November 2011

Comments

#Comment by Khorn (WMF) (talk | contribs)   00:34, 5 November 2011
+	 * @todo
+	 * - Can we just get the language from somewhere else to make this simpler?
+	 *

Yes.
getData_Raw( 'language' );
It's public and everything.

Two more things here: All this logic is not only combing through successful transaction results for data we have locally, it's doing it for specific transactions in the common gateway adapter. Please keep everything that is gateway-specific (and certainly everything that is transaction-specific) as far away from the common adapter as you possibly can, at all times.

Also: Once again, 'errors' is still a misnomer in the gateway objects. It should be RESPONSE codes. So, there's no guarantee they will be empty if the transaction was a success. This should all be renamed appropriately.

#Comment by Khorn (WMF) (talk | contribs)   18:40, 5 November 2011

Also: do_transaction already returns the result codes array in it's result, under the 'errors' key. Can you just pass the result handler the array of errors, straight from the do_transaction result?

#Comment by Awjrichards (talk | contribs)   21:35, 6 November 2011

Marking as OK as this is not a blocker for launch. Jeremy, please take note and we'll clean this up later.

Status & tagging log