jenkins-bot has submitted this change and it was merged. Change subject: Split staging from unstaging ......................................................................
Split staging from unstaging The combined stage_ helpers were muddying things, and were often only working correctly when used in the unstaged -> staged direction. Many of these functions were working on staged source data, when they should have been working on the unstaged data. This was mostly working by accident, because we copy the normalized data on top of both the staged and unstaged data before the staging code runs. Change-Id: If2d241fed6cc055123abfc97d58e25bdd34ae0f8 --- M adyen_gateway/adyen.adapter.php M amazon_gateway/amazon.adapter.php M gateway_common/GatewayPage.php M gateway_common/gateway.adapter.php M gateway_forms/RapidHtml.php M globalcollect_gateway/globalcollect.adapter.php M globalcollect_gateway/globalcollect_gateway.body.php M globalcollect_gateway/scripts/orphan_adapter.php M paypal_gateway/paypal.adapter.php M tests/Adapter/WorldPay/WorldPayTest.php M tests/GatewayPageTest.php M tests/GatewayValidationTest.php M worldpay_gateway/worldpay.adapter.php 13 files changed, 243 insertions(+), 282 deletions(-) Approvals: Ejegg: Looks good to me, approved jenkins-bot: Verified diff --git a/adyen_gateway/adyen.adapter.php b/adyen_gateway/adyen.adapter.php index 007eaa7..b2641aa 100644 --- a/adyen_gateway/adyen.adapter.php +++ b/adyen_gateway/adyen.adapter.php @@ -199,7 +199,7 @@ case 'donate': $formaction = $this->getGlobal( 'BaseURL' ) . '/hpp/pay.shtml'; $this->runAntifraudHooks(); - $this->addData( array ( 'risk_score' => $this->risk_score ) ); //this will also fire off staging again. + $this->addRequestData( array ( 'risk_score' => $this->risk_score ) ); //this will also fire off staging again. if ( $this->getValidationAction() != 'process' ) { // copied from base class. $this->log( "Failed pre-process checks for transaction type $transaction.", LOG_INFO ); @@ -516,44 +516,15 @@ protected function stage_language( $type = 'request' ) { */ - /** - * Stage: amount - * - * For example: JPY 1000.05 get changed to 100005. This need to be 100000. - * For example: JPY 1000.95 get changed to 100095. This need to be 100000. - * - * @param string $type request|response - */ - protected function stage_amount( $type = 'request' ) { - if ( !isset( $this->staged_data['amount'] ) || !isset( $this->staged_data['currency_code'] ) ) { - //can't do anything with amounts at all. Just go home. - return; - } - - switch ( $type ) { - case 'request': - if ( !DataValidator::is_fractional_currency( $this->staged_data['currency_code'] ) ) { - $this->staged_data['amount'] = floor( $this->staged_data['amount'] ); - } - - $this->staged_data['amount'] = $this->staged_data['amount'] * 100; - break; - - case 'response': - $this->staged_data['amount'] = $this->staged_data['amount'] / 100; - break; - } - } - - protected function stage_risk_score( $type = 'request' ) { + protected function stage_risk_score() { //This isn't smart enough to grab a new value here; //Late-arriving values have to trigger a restage via addData or //this will always equil the risk_score at the time of object //construction. Still need the formatting, though. - $this->staged_data['risk_score'] = ( string ) round( $this->staged_data['risk_score'] ); + $this->staged_data['risk_score'] = ( string ) round( $this->unstaged_data['risk_score'] ); } - protected function stage_hpp_signature( $type = 'request' ) { + protected function stage_hpp_signature() { $keys = array( 'amount', 'currency_code', @@ -577,7 +548,7 @@ $this->staged_data['hpp_signature'] = $this->calculateSignature( $sig_values ); } - protected function stage_billing_signature( $type = 'request' ) { + protected function stage_billing_signature() { $keys = array( 'street', 'city', diff --git a/amazon_gateway/amazon.adapter.php b/amazon_gateway/amazon.adapter.php index ebab94b..767b817 100644 --- a/amazon_gateway/amazon.adapter.php +++ b/amazon_gateway/amazon.adapter.php @@ -26,7 +26,7 @@ parent::__construct( $options ); if ($this->getData_Unstaged_Escaped( 'payment_method' ) == null ) { - $this->addData( + $this->addRequestData( array( 'payment_method' => 'amazon' ) ); } @@ -403,7 +403,7 @@ } } //TODO: consider prioritizing the session vars - $this->addData( $add_data ); //using the gateway's addData function restages everything + $this->addResponseData( $add_data ); //using the gateway's addData function restages everything $txnid = $this->dataObj->getVal_Escaped( 'gateway_txn_id' ); $email = $this->dataObj->getVal_Escaped( 'email' ); diff --git a/gateway_common/GatewayPage.php b/gateway_common/GatewayPage.php index 984baaa..f92dc29 100644 --- a/gateway_common/GatewayPage.php +++ b/gateway_common/GatewayPage.php @@ -327,7 +327,7 @@ $newAmount = floor( $usdAmount * $conversionRates[$defaultCurrency] ); } - $this->adapter->addData( array( + $this->adapter->addRequestData( array( 'amount' => $newAmount, 'currency_code' => $defaultCurrency ) ); diff --git a/gateway_common/gateway.adapter.php b/gateway_common/gateway.adapter.php index 51faed6..5afc88c 100644 --- a/gateway_common/gateway.adapter.php +++ b/gateway_common/gateway.adapter.php @@ -150,7 +150,7 @@ * Called in the constructor, this function should be used to define * pieces of default data particular to the gateway. It will be up to * the child class to poke the data through to the data object - * (probably with $this->addData()). + * (probably with $this->addRequestData()). * DO NOT set default payment information here (or anywhere, really). * That would be naughty. */ @@ -378,7 +378,7 @@ $this->account_config = $acctConfig[ $this->account_name ]; - $this->addData( array( + $this->addRequestData( array( 'gateway_account' => $this->account_name, ) ); } @@ -507,20 +507,18 @@ } /** - * A helper function to let us stash extra data after the form has been submitted. + * A helper function to let us stash extra data after the form has been submitted. * * @param array $dataArray An associative array of data. - * @param string $pipelineStage 'request' to the gateway or 'response' from the - * gateway depending on what you're actually doing. */ - public function addData( $dataArray, $pipelineStage = 'request' ) { + public function addRequestData( $dataArray ) { $this->dataObj->addData( $dataArray ); $calculated_fields = $this->dataObj->getCalculatedFields(); $data_fields = array_keys( $dataArray ); $data_fields = array_merge( $data_fields, $calculated_fields ); - foreach ( $data_fields as $value){ + foreach ( $data_fields as $value ) { $this->refreshGatewayValueFromSource( $value ); } @@ -530,8 +528,23 @@ //function, to calculate any other staged var. $changed_staged_vars = array_intersect( $this->staged_vars, $data_fields ); if ( count( $changed_staged_vars ) ) { - $this->stageData( $pipelineStage ); + $this->stageData(); } + } + + /** + * Stash and process response data. + * + * @param array $dataArray An associative array of data. + */ + public function addResponseData( $dataArray ) { + foreach ( $dataArray as $key => $value ) { + $this->staged_data[$key] = $value; + } + + $this->unstageData(); + + $this->dataObj->addData( $this->unstaged_data ); } /** @@ -1989,25 +2002,38 @@ } /** - * @param string $type Whatever types of staging you feel like having in your child class. - * ...but usually request and response. I think. + * Run any staging functions provided by the adapter */ - protected function stageData( $type = 'request' ) { - if ( $type === 'request' ){ - //reset from our normalized unstaged data so we never double-stage - $this->staged_data = $this->unstaged_data; - } + protected function stageData() { + // Copy data, the default is to not change the values. + //reset from our normalized unstaged data so we never double-stage + $this->staged_data = $this->unstaged_data; + + // This allows transactions to each stage different data. $this->defineStagedVars(); - //If we tried to piggyback off the same loop, all the vars wouldn't be ready, and some staging functions will require - //multiple variables. + foreach ( $this->staged_vars as $field ) { $function_name = 'stage_' . $field; - $this->executeIfFunctionExists( $function_name, $type ); + $this->executeIfFunctionExists( $function_name ); } // Format the staged data - if ($type === 'request'){ - $this->formatStagedData(); + $this->formatStagedData(); + } + + /** + * Run any unstaging functions to decode processor responses + */ + protected function unstageData() { + // Copy from staged data, + $this->unstaged_data = $this->staged_data; + + // FIXME: we should have a list of unstaged_vars for each transaction. + $this->defineStagedVars(); + + foreach ( $this->staged_vars as $field ) { + $function_name = 'unstage_' . $field; + $this->executeIfFunctionExists( $function_name ); } } @@ -2047,57 +2073,86 @@ } } + /** + * Stage: amount + * + * For example: JPY 1000.05 get changed to 100005. This need to be 100000. + * For example: JPY 1000.95 get changed to 100095. This need to be 100000. + */ + protected function stage_amount() { + if ( !$this->getData_Unstaged_Escaped( 'amount' ) + || !$this->getData_Unstaged_Escaped( 'currency_code' ) + ) { + //can't do anything with amounts at all. Just go home. + unset( $this->staged_data['amount'] ); + return; + } + + $amount = $this->getData_Unstaged_Escaped( 'amount' ); + if ( !DataValidator::is_fractional_currency( $this->getData_Unstaged_Escaped( 'currency_code' ) ) ) { + $amount = floor( $amount ); + } + + $this->staged_data['amount'] = $amount * 100; + } + + protected function unstage_amount() { + $this->unstaged_data['amount'] = $this->getData_Staged( 'amount' ) / 100; + } + /** - * Stage the street address. In the event that there isn't anything in - * there, we need to send something along so that AVS checks get triggered - * at all. + * Stage the street address + * + * In the event that there isn't anything in there, we need to send + * something along so that AVS checks get triggered at all. + * * The zero is intentional: Allegedly, Some banks won't perform the check * if the address line contains no numerical data. - * @param string $type request|response */ - protected function stage_street( $type = 'request' ) { - if ( !isset( $this->staged_data['street'] ) ) { + protected function stage_street() { + if ( !isset( $this->unstaged_data['street'] ) ) { //nothing to do. return; } - if ( $type === 'request' ){ - $is_garbage = false; - $street = trim( $this->staged_data['street'] ); - ( strlen( $street ) === 0 ) ? $is_garbage = true : null; - ( !DataValidator::validate_not_just_punctuation( $street ) ) ? $is_garbage = true : null; - if ( $is_garbage ){ - $this->staged_data['street'] = 'N0NE PROVIDED'; //The zero is intentional. See function comment. - } + $is_garbage = false; + $street = trim( $this->unstaged_data['street'] ); + ( strlen( $street ) === 0 ) ? $is_garbage = true : null; + ( !DataValidator::validate_not_just_punctuation( $street ) ) ? $is_garbage = true : null; + + if ( $is_garbage ){ + $this->staged_data['street'] = 'N0NE PROVIDED'; //The zero is intentional. See function comment. } } /** - * Stage the zip. In the event that there isn't anything in - * there, we need to send something along so that AVS checks get triggered - * at all. - * @param string $type request|response + * Stage the zip / postal code + * + * In the event that there isn't anything in there, we need to send + * something along so that AVS checks get triggered at all. */ - protected function stage_zip( $type = 'request' ) { - if ( !isset( $this->staged_data['zip'] ) ) { + protected function stage_zip() { + if ( !isset( $this->unstaged_data['zip'] ) ) { //nothing to do. return; } - if ( $type === 'request' && strlen( trim( $this->staged_data['zip'] ) ) === 0 ){ + if ( strlen( trim( $this->unstaged_data['zip'] ) ) === 0 ){ //it would be nice to check for more here, but the world has some //straaaange postal codes... - $this->staged_data['zip'] = '0'; + $this->unstaged_data['zip'] = '0'; } //country-based zip grooming to make AVS (marginally) happy - switch ($this->getData_Staged( 'country' ) ){ + switch ($this->getData_Unstaged_Escaped( 'country' ) ){ case 'CA': //Canada goes "A0A 0A0" - $this->staged_data['zip'] = strtoupper( $this->staged_data['zip'] ); + $this->staged_data['zip'] = strtoupper( $this->unstaged_data['zip'] ); //In the event that they only forgot the space, help 'em out. $regex = '/[A-Z]{1}\d{1}[A-Z]{1}\d{1}[A-Z]{1}\d{1}/'; - if ( strlen( $this->staged_data['zip'] ) === 6 && preg_match( $regex, $this->staged_data['zip'] ) ) { - $zip = $this->staged_data['zip']; + if ( strlen( $this->staged_data['zip'] ) === 6 + && preg_match( $regex, $this->unstaged_data['zip'] ) + ) { + $zip = $this->unstaged_data['zip']; $this->staged_data['zip'] = substr( $zip, 0, 3 ) . ' ' . substr( $zip, 3, 3 ); } break; @@ -3272,7 +3327,7 @@ $_SESSION[$gateway_ident . 'EditToken'] = $unsalted; $salted = $this->token_getSaltedSessionToken(); - $this->addData( array ( 'token' => $salted ) ); + $this->addRequestData( array ( 'token' => $salted ) ); } /** @@ -3322,7 +3377,7 @@ // match token if ( !$this->dataObj->isSomething( 'token' ) ) { - $this->addData( array ( 'token' => $token ) ); + $this->addRequestData( array ( 'token' => $token ) ); } $token_check = $this->getData_Unstaged_Escaped( 'token' ); @@ -3414,13 +3469,13 @@ return; } else if ( $this->session_getLastRapidHTMLForm() ) { //This will take care of it if this is an ajax request, or a 3rd party return hit $new_ff = $this->session_getLastRapidHTMLForm(); - $this->addData( array ( 'ffname' => $new_ff ) ); + $this->addRequestData( array ( 'ffname' => $new_ff ) ); //and debug log a little $this->log( "Setting form to last successful ('$new_ff')", LOG_DEBUG ); } else if ( GatewayFormChooser::isValidForm( $ffname . "-$country", $country, $currency, $payment_method, $payment_submethod, $recurring, $gateway ) ) { //if the country-specific version exists, use that. - $this->addData( array ( 'ffname' => $ffname . "-$country" ) ); + $this->addRequestData( array ( 'ffname' => $ffname . "-$country" ) ); //I'm only doing this for serious legacy purposes. This mess needs to stop itself. To help with the mess-stopping... $message = "ffname '$ffname' was invalid, but the country-specific '$ffname-$country' works. utm_source = '$utm', referrer = '$ref'"; @@ -3428,7 +3483,7 @@ } else { //Invalid form. Go get one that is valid, and squak in the error logs. $new_ff = GatewayFormChooser::getOneValidForm( $country, $currency, $payment_method, $payment_submethod, $recurring, $gateway ); - $this->addData( array ( 'ffname' => $new_ff ) ); + $this->addRequestData( array ( 'ffname' => $new_ff ) ); //now construct a useful error message $this->log( @@ -3701,7 +3756,7 @@ } //tell DonationData about it - $this->addData( array ( 'order_id' => $id ) ); + $this->addRequestData( array ( 'order_id' => $id ) ); return $id; } diff --git a/gateway_forms/RapidHtml.php b/gateway_forms/RapidHtml.php index d6cff83..b79cd93 100644 --- a/gateway_forms/RapidHtml.php +++ b/gateway_forms/RapidHtml.php @@ -515,7 +515,7 @@ $params = array_merge( $preserve, $params ); } //If this is just the one thing, we might move this inside DonationData for clarity's sake... - $this->gateway->addData( array ( 'ffname_retry' => GatewayFormChooser::buildPaymentsFormURL( $back_form, $params ) ) ); + $this->gateway->addRequestData( array ( 'ffname_retry' => GatewayFormChooser::buildPaymentsFormURL( $back_form, $params ) ) ); } } else { //No special type... let's add this to the form stack and call it good. diff --git a/globalcollect_gateway/globalcollect.adapter.php b/globalcollect_gateway/globalcollect.adapter.php index 8fea4fd..de70aa6 100644 --- a/globalcollect_gateway/globalcollect.adapter.php +++ b/globalcollect_gateway/globalcollect.adapter.php @@ -326,7 +326,7 @@ 'effort_id' => '1', ); - $this->addData( $defaults ); + $this->addRequestData( $defaults ); } /** @@ -1108,8 +1108,11 @@ } $is_orphan = false; - if ( count( $qsResults ) ){ //nothing unusual here. - $this->addData( $qsResults ); + if ( count( $qsResults ) ){ + // Nothing unusual here. Oh, except we are reading query parameters from + // what we hope is a redirect back from the processor, caused by an earlier + // transaction. + $this->addResponseData( $qsResults ); $logmsg = 'CVV Result from querystring: ' . $this->getData_Unstaged_Escaped( 'cvv_result' ); $logmsg .= ', AVS Result from querystring: ' . $this->getData_Unstaged_Escaped( 'avs_result' ); $this->log( $logmsg ); @@ -1155,7 +1158,7 @@ } } } - $this->addData( $xmlResults ); + $this->addResponseData( $xmlResults ); $logmsg = 'CVV Result from XML: ' . $this->getData_Unstaged_Escaped( 'cvv_result' ); $logmsg .= ', AVS Result from XML: ' . $this->getData_Unstaged_Escaped( 'avs_result' ); $this->log( $logmsg ); @@ -1266,7 +1269,7 @@ } if ( count( $addme ) ){ - $this->addData( $addme ); + $this->addResponseData( $addme ); } } @@ -1454,7 +1457,7 @@ $this->log( "inside " . $data['ORDERID'] ); $this->normalizeOrderID( $data['ORDERID'] ); $this->log( print_r( $this->getOrderIDMeta(), true ) ); - $this->addData( array ( 'order_id' => $data['ORDERID'] ) ); + $this->addRequestData( array ( 'order_id' => $data['ORDERID'] ) ); $this->log( print_r( $this->getOrderIDMeta(), true ) ); $this->session_addDonorData(); } @@ -1859,39 +1862,38 @@ ); } - protected function stage_language( $type = 'request' ) { - $language = strtolower( $this->staged_data['language'] ); + protected function stage_language() { + $language = strtolower( $this->getData_Unstaged_Escaped( 'language' ) ); - switch ( $type ) { - case 'request': - if ( !in_array( $language, $this->getAvailableLanguages() ) ) { - $fallbacks = Language::getFallbacksFor( $language ); - foreach ( $fallbacks as $fallback ) { - if ( in_array( $fallback, $this->getAvailableLanguages() ) ) { - $language = $fallback; - break; - } - } + if ( !in_array( $language, $this->getAvailableLanguages() ) ) { + $fallbacks = Language::getFallbacksFor( $language ); + foreach ( $fallbacks as $fallback ) { + if ( in_array( $fallback, $this->getAvailableLanguages() ) ) { + $language = $fallback; + break; } + } + } - if ( !in_array( $language, $this->getAvailableLanguages() ) ){ - $language = 'en'; - } + if ( !in_array( $language, $this->getAvailableLanguages() ) ){ + $language = 'en'; + } - if ( $language === 'zh' ) { //Handles GC's mutant Chinese code. - $language = 'sc'; - } - - break; - case 'response': - if ( $language === 'sc' ){ - $language = 'zh'; - } - break; + if ( $language === 'zh' ) { //Handles GC's mutant Chinese code. + $language = 'sc'; } $this->staged_data['language'] = $language; + } + protected function unstage_language() { + $language = strtolower( $this->staged_data['language'] ); + + if ( $language === 'sc' ){ + $language = 'zh'; + } + + $this->unstaged_data['language'] = $language; } /** @@ -1934,43 +1936,11 @@ } /** - * Stage: amount - * - * For example: JPY 1000.05 get changed to 100005. This need to be 100000. - * For example: JPY 1000.95 get changed to 100095. This need to be 100000. - * - * @param string $type request|response - */ - protected function stage_amount( $type = 'request' ) { - if ( !isset( $this->staged_data['amount'] ) || !isset( $this->staged_data['currency_code'] ) ) { - //can't do anything with amounts at all. Just go home. - return; - } - switch ( $type ) { - case 'request': - - if ( !DataValidator::is_fractional_currency( $this->staged_data['currency_code'] ) ) { - $this->staged_data['amount'] = floor( $this->staged_data['amount'] ); - } - - $this->staged_data['amount'] = $this->staged_data['amount'] * 100; - - break; - case 'response': - $this->staged_data['amount'] = $this->staged_data['amount'] / 100; - break; - } - } - - /** * Stage: card_num - * - * @param string $type request|response */ - protected function stage_card_num( $type = 'request' ) { - //I realize that the $type isn't used. Voodoo. - if ( array_key_exists( 'card_num', $this->staged_data ) ) { - $this->staged_data['card_num'] = str_replace( ' ', '', $this->staged_data['card_num'] ); + protected function stage_card_num() { + if ( array_key_exists( 'card_num', $this->unstaged_data ) ) { + $this->staged_data['card_num'] = str_replace( ' ', '', $this->unstaged_data['card_num'] ); } } @@ -1979,7 +1949,7 @@ * Stages the payment product ID for GC. * Not what I had in mind to begin with, but this *completely* blew up. */ - public function stage_payment_product( $type = 'request' ) { + public function stage_payment_product() { //cc used to look in card_type, but that's been an alias for payment_submethod for a while. DonationData takes care of it. $payment_method = array_key_exists( 'payment_method', $this->staged_data ) ? $this->staged_data['payment_method'] : false; $payment_submethod = array_key_exists( 'payment_submethod', $this->staged_data ) ? $this->staged_data['payment_submethod'] : false; @@ -1997,10 +1967,6 @@ 'discover' => '128', 'cb' => '130', ); - - if ( $type === 'response' ) { - $types = array_flip( $types ); - } if ( (!is_null( $payment_submethod ) ) && array_key_exists( $payment_submethod, $types ) ) { $this->staged_data['payment_product'] = $types[$payment_submethod]; @@ -2034,6 +2000,7 @@ } } + // FIXME: that's one hell of a staging function. Move this to a do_transaction helper. if ( $enable3ds ) { $this->log( "3dSecure enabled for $currency in $country" ); $this->transactions['INSERT_ORDERWITHPAYMENT']['values']['AUTHENTICATIONINDICATOR'] = '1'; @@ -2056,43 +2023,37 @@ * Stage branch_code for Direct Debit. * Check the data constraints, and zero-pad out to that number where possible. * Exceptions for the defaults are set in stage_country so we can see them all in the same place - * @param string $type request|response */ - protected function stage_branch_code( $type = 'request' ) { - if ( $type === 'request' && isset( $this->staged_data['branch_code'] ) ) { - $newval = DataValidator::getZeroPaddedValue( $this->staged_data['branch_code'], $this->dataConstraints['branch_code']['length'] ); - if ( $newval ) { - $this->staged_data['branch_code'] = $newval; - } - } + protected function stage_branch_code() { + $this->stageAndZeroPad( 'branch_code' ); } /** * Stage bank_code for Direct Debit. * Check the data constraints, and zero-pad out to that number where possible. * Exceptions for the defaults are set in stage_country so we can see them all in the same place - * @param string $type request|response */ - protected function stage_bank_code( $type = 'request' ) { - if ( $type === 'request' && isset( $this->staged_data['bank_code'] ) ) { - $newval = DataValidator::getZeroPaddedValue( $this->staged_data['bank_code'], $this->dataConstraints['bank_code']['length'] ); - if ( $newval ) { - $this->staged_data['bank_code'] = $newval; - } - } + protected function stage_bank_code() { + $this->stageAndZeroPad( 'bank_code' ); } /** * Stage account_number for Direct Debit. * Check the data constraints, and zero-pad out to that number where possible. * Exceptions for the defaults are set in stage_country so we can see them all in the same place - * @param string $type request|response */ - protected function stage_account_number( $type = 'request' ) { - if ( $type === 'request' && isset( $this->staged_data['account_number'] ) ) { - $newval = DataValidator::getZeroPaddedValue( $this->staged_data['account_number'], $this->dataConstraints['account_number']['length'] ); + protected function stage_account_number() { + $this->stageAndZeroPad( 'account_number' ); + } + + /** + * Helper to stage a zero-padded number + */ + protected function stageAndZeroPad( $key ) { + if ( isset( $this->unstaged_data[$key] ) ) { + $newval = DataValidator::getZeroPaddedValue( $this->unstaged_data[$key], $this->dataConstraints[$key]['length'] ); if ( $newval ) { - $this->staged_data['account_number'] = $newval; + $this->staged_data[$key] = $newval; } } } @@ -2101,9 +2062,8 @@ * Stage: setupStagePaymentMethodForDirectDebit * * @param string $payment_submethod - * @param string $type request|response */ - protected function setupStagePaymentMethodForDirectDebit( $payment_submethod, $type = 'request' ) { + protected function setupStagePaymentMethodForDirectDebit( $payment_submethod ) { // DATECOLLECT is required on all Direct Debit $this->addKeyToTransaction('DATECOLLECT'); @@ -2123,9 +2083,8 @@ * Stage: setupStagePaymentMethodForEWallets * * @param string $payment_submethod - * @param string $type request|response */ - protected function setupStagePaymentMethodForEWallets( $payment_submethod, $type = 'request' ) { + protected function setupStagePaymentMethodForEWallets( $payment_submethod ) { // DESCRIPTOR is required on WebMoney, assuming it is required for all. $this->addKeyToTransaction('DESCRIPTOR'); @@ -2141,8 +2100,6 @@ /** * Stage: payment_method * - * @param string $type request|response - * * @todo * - Need to implement this for credit card if necessary * - ISSUERID will need to provide a dropdown for rtbt_eps and rtbt_ideal. @@ -2150,10 +2107,9 @@ * - DATECOLLECT is using gmdate('Ymd') * - DIRECTDEBITTEXT will need to be translated. This is what appears on the bank statement for donations for a client. This is hardcoded to: Wikimedia Foundation */ - protected function stage_payment_method( $type = 'request' ) { - - $payment_method = array_key_exists( 'payment_method', $this->staged_data ) ? $this->staged_data['payment_method']: false; - $payment_submethod = array_key_exists( 'payment_submethod', $this->staged_data ) ? $this->staged_data['payment_submethod']: false; + protected function stage_payment_method() { + $payment_method = array_key_exists( 'payment_method', $this->unstaged_data ) ? $this->unstaged_data['payment_method']: false; + $payment_submethod = array_key_exists( 'payment_submethod', $this->unstaged_data ) ? $this->unstaged_data['payment_submethod']: false; //Having to front-load the country in the payment submethod is pretty lame. //If we don't have one deliberately set... @@ -2165,19 +2121,19 @@ } } - // These will be grouped and ordred by payment product id + // These will be grouped and ordered by payment product id switch ( $payment_submethod ) { /* Bank transfer */ case 'bt': // Brazil - if ( $this->staged_data['country'] == 'BR' ) { + if ( $this->unstaged_data['country'] == 'BR' ) { $this->dataConstraints['direct_debit_text']['city'] = 50; } // Korea - Manual does not specify North or South - if ( $this->staged_data['country'] == 'KR' ) { + if ( $this->unstaged_data['country'] == 'KR' ) { $this->dataConstraints['direct_debit_text']['city'] = 50; } break; @@ -2241,10 +2197,10 @@ switch ($payment_method) { case 'dd': - $this->setupStagePaymentMethodForDirectDebit( $payment_submethod, $type); + $this->setupStagePaymentMethodForDirectDebit( $payment_submethod ); break; case 'ew': - $this->setupStagePaymentMethodForEWallets( $payment_submethod, $type); + $this->setupStagePaymentMethodForEWallets( $payment_submethod ); break; } } @@ -2253,11 +2209,9 @@ * Stage: recurring * Adds the recurring payment pieces to the structure of * INSERT_ORDERWITHPAYMENT if the recurring field is populated. - * - * @param string $type request|response */ - protected function stage_recurring( $type = 'request' ){ - if ( ! $this->getData_Staged( 'recurring' ) ){ + protected function stage_recurring(){ + if ( !$this->getData_Unstaged_Escaped( 'recurring' ) ) { return; } else { $this->transactions['INSERT_ORDERWITHPAYMENT']['request']['REQUEST']['PARAMS']['ORDER'][] = 'ORDERTYPE'; @@ -2269,15 +2223,9 @@ * Stage: country * This should be a catch-all for establishing weird country-based rules. * Right now, we only have the one, but there could be more here later. - * - * @param string $type request|response */ - protected function stage_country( $type = 'request' ){ - if ( $type != 'request' ){ - return; //nothing to do here. - } - - switch ( $this->getData_Staged( 'country' ) ){ + protected function stage_country() { + switch ( $this->getData_Unstaged_Escaped( 'country' ) ){ case 'AR' : $this->transactions['INSERT_ORDERWITHPAYMENT']['request']['REQUEST']['PARAMS']['ORDER'][] = 'USAGETYPE'; $this->transactions['INSERT_ORDERWITHPAYMENT']['request']['REQUEST']['PARAMS']['ORDER'][] = 'PURCHASETYPE'; @@ -2287,24 +2235,22 @@ } } - protected function stage_contribution_tracking_id( $type = 'request' ) { - $ctid = $this->staged_data['contribution_tracking_id']; - if ( $type === 'request' ) { - //append timestamp to ctid - $ctid .= '.' . (( microtime( true ) * 1000 ) % 100000); //least significant five - } elseif ( $type === 'response' ) { - $ctid = explode( '.', $ctid ); - $ctid = $ctid[0]; - } + protected function stage_contribution_tracking_id() { + $ctid = $this->unstaged_data['contribution_tracking_id']; + //append timestamp to ctid + $ctid .= '.' . (( microtime( true ) * 1000 ) % 100000); //least significant five $this->staged_data['contribution_tracking_id'] = $ctid; } - protected function stage_fiscal_number( $type = 'request' ) { - if ( $type != 'request' ){ - return; //nothing to do here. - } + protected function unstage_contribution_tracking_id() { + $ctid = $this->staged_data['contribution_tracking_id']; + $ctid = explode( '.', $ctid ); + $ctid = $ctid[0]; + $this->unstaged_data['contribution_tracking_id'] = $ctid; + } - $this->staged_data['fiscal_number'] = preg_replace( "/[\.\/\-]/", "", $this->getData_Staged( 'fiscal_number' ) ); + protected function stage_fiscal_number() { + $this->staged_data['fiscal_number'] = preg_replace( "/[\.\/\-]/", "", $this->getData_Unstaged_Escaped( 'fiscal_number' ) ); } /** @@ -2327,32 +2273,23 @@ /** * Stage: returnto - * - * @param string $type request|response */ - protected function stage_returnto( $type = 'request' ) { + protected function stage_returnto() { + // Get the default returnto + $returnto = $this->getData_Unstaged_Escaped( 'returnto' ); - if ( $type === 'request' ) { - - // Get the default returnto - $returnto = $this->getData_Staged( 'returnto' ); - - if ( $this->getData_Unstaged_Escaped( 'payment_method' ) === 'cc' ){ - - // Add order ID to the returnto URL, only if it's not already there. - //TODO: This needs to be more robust (like actually pulling the - //qstring keys, resetting the values, and putting it all back) - //but for now it'll keep us alive. - if ( $this->getOrderIDMeta( 'generate' ) && !is_null( $returnto ) && !strpos( $returnto, 'order_id' ) ) { - $queryArray = array( 'order_id' => $this->staged_data['order_id'] ); - $this->staged_data['returnto'] = wfAppendQuery( $returnto, $queryArray ); - } - - } else { - - // Do we want to set this here? - $this->staged_data['returnto'] = $this->getThankYouPage(); + if ( $this->getData_Unstaged_Escaped( 'payment_method' ) === 'cc' ) { + // Add order ID to the returnto URL, only if it's not already there. + //TODO: This needs to be more robust (like actually pulling the + //qstring keys, resetting the values, and putting it all back) + //but for now it'll keep us alive. + if ( $this->getOrderIDMeta( 'generate' ) && !is_null( $returnto ) && !strpos( $returnto, 'order_id' ) ) { + $queryArray = array( 'order_id' => $this->unstaged_data['order_id'] ); + $this->staged_data['returnto'] = wfAppendQuery( $returnto, $queryArray ); } + } else { + // FIXME: Do we want to set this here? + $this->staged_data['returnto'] = $this->getThankYouPage(); } } diff --git a/globalcollect_gateway/globalcollect_gateway.body.php b/globalcollect_gateway/globalcollect_gateway.body.php index 73a492c..f18cd72 100644 --- a/globalcollect_gateway/globalcollect_gateway.body.php +++ b/globalcollect_gateway/globalcollect_gateway.body.php @@ -50,7 +50,7 @@ and !$this->adapter->getPaymentSubmethod() ) { // Synthesize a submethod based on the country. $country_code = strtolower( $this->adapter->getData_Unstaged_Escaped( 'country' ) ); - $this->adapter->addData( array( + $this->adapter->addRequestData( array( 'payment_submethod' => "dd_{$country_code}", ) ); } diff --git a/globalcollect_gateway/scripts/orphan_adapter.php b/globalcollect_gateway/scripts/orphan_adapter.php index 356e3f0..0e0806b 100644 --- a/globalcollect_gateway/scripts/orphan_adapter.php +++ b/globalcollect_gateway/scripts/orphan_adapter.php @@ -80,8 +80,8 @@ $this->revalidate(); } - public function addData( $dataArray, $pipelineStage = 'request' ) { - parent::addData( $dataArray, $pipelineStage ); + public function addRequestData( $dataArray ) { + parent::addRequestData( $dataArray ); $this->reAddHardData(); } diff --git a/paypal_gateway/paypal.adapter.php b/paypal_gateway/paypal.adapter.php index a6fbed2..7017308 100644 --- a/paypal_gateway/paypal.adapter.php +++ b/paypal_gateway/paypal.adapter.php @@ -26,7 +26,7 @@ parent::__construct( $options ); if ($this->getData_Unstaged_Escaped( 'payment_method' ) == null ) { - $this->addData( + $this->addRequestData( array( 'payment_method' => 'paypal' ) ); } @@ -224,13 +224,13 @@ ); } - protected function stage_recurring_length( $mode = 'request' ) { + protected function stage_recurring_length() { if ( array_key_exists( 'recurring_length', $this->staged_data ) && !$this->staged_data['recurring_length'] ) { unset( $this->staged_data['recurring_length'] ); } } - protected function stage_locale( $mode = 'request' ) { + protected function stage_locale() { $supported_countries = array( 'AU', 'AT', @@ -266,13 +266,13 @@ 'zh_TW', ); - if ( in_array( $this->staged_data['country'], $supported_countries ) ) { - $this->staged_data['locale'] = $this->staged_data['country']; + if ( in_array( $this->unstaged_data['country'], $supported_countries ) ) { + $this->staged_data['locale'] = $this->unstaged_data['country']; } - $fallbacks = Language::getFallbacksFor( strtolower( $this->staged_data['language'] ) ); + $fallbacks = Language::getFallbacksFor( strtolower( $this->unstaged_data['language'] ) ); foreach ( $fallbacks as $lang ) { - $locale = "{$this->staged_data['language']}_{$this->staged_data['country']}"; + $locale = "{$this->unstaged_data['language']}_{$this->unstaged_data['country']}"; if ( in_array( $locale, $supported_full_locales ) ) { $this->staged_data['locale'] = $locale; return; diff --git a/tests/Adapter/WorldPay/WorldPayTest.php b/tests/Adapter/WorldPay/WorldPayTest.php index e0eb25f..2d79890 100644 --- a/tests/Adapter/WorldPay/WorldPayTest.php +++ b/tests/Adapter/WorldPay/WorldPayTest.php @@ -243,7 +243,7 @@ $this->assertTrue( $gateway->getCVVResult(), 'getCVVResult not passing somebody with a match.' ); //and now, for fun, test a wrong code. - $gateway->addData( array ( 'cvv_result' => '2' ), 'response' ); + $gateway->addResponseData( array ( 'cvv_result' => '2' ) ); $this->assertFalse( $gateway->getCVVResult(), 'getCVVResult not failing somebody with garbage.' ); } diff --git a/tests/GatewayPageTest.php b/tests/GatewayPageTest.php index ba1f3bc..6050855 100644 --- a/tests/GatewayPageTest.php +++ b/tests/GatewayPageTest.php @@ -29,7 +29,7 @@ public function setUp() { $this->page = new TestingGatewayPage(); $this->adapter = new TestingGenericAdapter(); - $this->adapter->addData( array( + $this->adapter->addRequestData( array( 'amount' => '120', 'currency_code' => 'SGD' ) ); $this->adapter->errorsForRevalidate[0] = array( 'currency_code' => 'blah' ); diff --git a/tests/GatewayValidationTest.php b/tests/GatewayValidationTest.php index 77315a4..7b22ba1 100644 --- a/tests/GatewayValidationTest.php +++ b/tests/GatewayValidationTest.php @@ -44,7 +44,7 @@ } public function testPassesValidation() { - $this->adapter->addData( array( + $this->adapter->addRequestData( array( 'amount' => '2.00', 'country' => 'US', 'currency' => 'USD', @@ -57,7 +57,7 @@ } public function testLowAmountError() { - $this->adapter->addData( array( + $this->adapter->addRequestData( array( 'amount' => '1.99', 'country' => 'US', 'currency' => 'USD', @@ -72,7 +72,7 @@ } public function testHighAmountError() { - $this->adapter->addData( array( + $this->adapter->addRequestData( array( 'amount' => '100.99', 'country' => 'US', 'currency' => 'USD', @@ -87,7 +87,7 @@ } public function testCurrencyCodeError() { - $this->adapter->addData( array( + $this->adapter->addRequestData( array( 'amount' => '2.99', 'country' => 'US', 'currency' => 'ZZZ', @@ -108,7 +108,7 @@ 'wgDonationInterfaceForbiddenCountries' => array( 'XX' ) ) ); - $this->adapter->addData( array( + $this->adapter->addRequestData( array( 'amount' => '2.99', 'country' => 'XX', 'currency' => 'USD', @@ -123,7 +123,7 @@ } public function testEmailError() { - $this->adapter->addData( array( + $this->adapter->addRequestData( array( 'amount' => '2.99', 'currency' => 'USD', 'email' => 'foo', @@ -138,7 +138,7 @@ } public function testSpuriousCcError() { - $this->adapter->addData( array( + $this->adapter->addRequestData( array( 'amount' => '2.99', 'currency' => 'USD', 'fname' => '4111111111111111', @@ -153,7 +153,7 @@ } public function testMissingFieldError() { - $this->adapter->addData( array( + $this->adapter->addRequestData( array( 'amount' => '2.99', ) ); diff --git a/worldpay_gateway/worldpay.adapter.php b/worldpay_gateway/worldpay.adapter.php index 6e167fd..7d30a17 100644 --- a/worldpay_gateway/worldpay.adapter.php +++ b/worldpay_gateway/worldpay.adapter.php @@ -300,7 +300,7 @@ } function setGatewayDefaults() { - $this->addData( array( + $this->addRequestData( array( 'region_code' => 0 // TODO: geolocating this into the right region... )); } @@ -762,7 +762,7 @@ break; case 'AuthorizePaymentForFraud': - $this->addData( array( 'cvv' => $this->get_cvv() ) ); + $this->addRequestData( array( 'cvv' => $this->get_cvv() ) ); $this->store_cvv_in_session( null ); // Remove the CVV from the session return parent::do_transaction( $transaction ); break; @@ -843,7 +843,7 @@ $emptyVars[] = $theirs; } } - $self->addData( $addme, 'response' ); + $self->addResponseData( $addme ); return $emptyVars; }; $setFailOnEmpty = function( $emptyVars ) use ( $response, $self ) { @@ -935,7 +935,7 @@ return $headers; } - protected function stage_returnto( $type = 'request' ) { + protected function stage_returnto() { global $wgServer, $wgArticlePath; $this->staged_data['returnto'] = str_replace( @@ -945,39 +945,37 @@ ); } - protected function stage_wp_acctname( $type = 'request' ) { + protected function stage_wp_acctname() { $this->staged_data['wp_acctname'] = implode( ' ', array( $this->getData_Unstaged_Escaped( 'fname' ), $this->getData_Unstaged_Escaped( 'lname' ) )); } - protected function stage_iso_currency_id( $type = 'request' ) { + protected function stage_iso_currency_id() { $currency = $this->getData_Unstaged_Escaped( 'currency_code' ); if ( array_key_exists( $currency, self::$CURRENCY_CODES ) ) { $this->staged_data['iso_currency_id'] = self::$CURRENCY_CODES[$currency]; } } - protected function stage_payment_submethod( $type = 'request' ) { - if ( $type == 'response' ) { - $paymentMethod = $this->getData_Unstaged_Escaped( 'payment_method' ); - $paymentSubmethod = $this->getData_Unstaged_Escaped( 'payment_submethod' ); - if ( $paymentMethod == 'cc' ) { - if ( array_key_exists( $paymentSubmethod, self::$CARD_TYPES ) ) { - $this->unstaged_data['payment_submethod'] = self::$CARD_TYPES[$paymentSubmethod]; - } + protected function unstage_payment_submethod() { + $paymentMethod = $this->getData_Staged( 'payment_method' ); + $paymentSubmethod = $this->getData_Staged( 'payment_submethod' ); + if ( $paymentMethod == 'cc' ) { + if ( array_key_exists( $paymentSubmethod, self::$CARD_TYPES ) ) { + $this->unstaged_data['payment_submethod'] = self::$CARD_TYPES[$paymentSubmethod]; } } } - protected function stage_merchant_reference_2( $type = 'request' ) { + protected function stage_merchant_reference_2() { $email = $this->getData_Unstaged_Escaped( 'email' ); $alphanumeric = preg_replace('/[^0-9a-zA-Z]/', ' ', $email); $this->staged_data['merchant_reference_2'] = $alphanumeric; } - protected function stage_narrative_statement_1( $type = 'request' ) { + protected function stage_narrative_statement_1() { $this->staged_data['narrative_statement_1'] = WmfFramework::formatMessage( 'donate_interface-statement', $this->getData_Unstaged_Escaped( 'contribution_tracking_id' ) -- To view, visit https://gerrit.wikimedia.org/r/189615 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: If2d241fed6cc055123abfc97d58e25bdd34ae0f8 Gerrit-PatchSet: 7 Gerrit-Project: mediawiki/extensions/DonationInterface Gerrit-Branch: master Gerrit-Owner: Awight <awi...@wikimedia.org> Gerrit-Reviewer: AndyRussG <andrew.green...@gmail.com> Gerrit-Reviewer: Awight <awi...@wikimedia.org> Gerrit-Reviewer: Ejegg <eeggles...@wikimedia.org> Gerrit-Reviewer: Katie Horn <kh...@wikimedia.org> Gerrit-Reviewer: Ssmith <ssm...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits