r99058 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r99057‎ | r99058 | r99059 >
Date:23:02, 5 October 2011
Author:khorn
Status:ok
Tags:fundraising 
Comment:
Even more bug fixes and additions for DonationInterface:
Made the old unit tests work again, changed some things around so the transaction data wasn't killing itself off anymore.
Also, added a line to kill all session data if we're not trying to save it, in do_transaction.
Yes, this is still followup r98498
Modified paths:
  • /branches/fundraising/extensions/DonationInterface/donationinterface.php (modified) (history)
  • /branches/fundraising/extensions/DonationInterface/gateway_common/DonationData.php (modified) (history)
  • /branches/fundraising/extensions/DonationInterface/gateway_common/gateway.adapter.php (modified) (history)
  • /branches/fundraising/extensions/DonationInterface/globalcollect_gateway/globalcollect.adapter.php (modified) (history)
  • /branches/fundraising/extensions/DonationInterface/globalcollect_gateway/globalcollect_resultswitcher.body.php (modified) (history)
  • /branches/fundraising/extensions/DonationInterface/payflowpro_gateway/payflowpro.adapter.php (modified) (history)
  • /branches/fundraising/extensions/DonationInterface/payflowpro_gateway/payflowpro_gateway.body.php (modified) (history)
  • /branches/fundraising/extensions/DonationInterface/tests/Adapter/GatewayAdapterTestCase.php (modified) (history)

Diff [purge]

Index: branches/fundraising/extensions/DonationInterface/donationinterface.php
——@@ -135,6 +135,6 @@
136136 $wgDonationInterfaceRetrySeconds = 5;
137137
138138 function efDonationInterfaceUnitTests( &$files ) {
139 - //$files[] = dirname( __FILE__ ) . '/tests/AllTests.php';
 139+ $files[] = dirname( __FILE__ ) . '/tests/AllTests.php';
140140 return true;
141141 }
Index: branches/fundraising/extensions/DonationInterface/tests/Adapter/GatewayAdapterTestCase.php
——@@ -24,13 +24,17 @@
2525 require_once dirname( dirname( __FILE__ ) ) . DIRECTORY_SEPARATOR . 'DonationInterfaceTestCase.php';
2626
2727 /**
28 - * TODO: Something.
29 - * Something roughly test-shaped. Here.
30 - * ...to be more precise: Test that ALL the gateway adapters (Yes: All two of them)
31 - * are building the XML we think they are, and that they can process sample
32 - * return XML the way we think they should.
 28+ * TODO: Test everything.
 29+ * Make sure all the basic functions in the gateway_adapter are tested here.
 30+ * Also, the extras and their hooks firing properly and... that the fail score
 31+ * they give back is acted upon in the way we think it does.
 32+ * Hint: For that mess, use GatewayAdapter's $debugarray
3333 *
34 - * TODO: Then, write all the other tests as well. :|
 34+ * Also, note that it barely makes sense to test the functions that need to be
 35+ * defined in each gateway as per the abstract class. If we did that here, we'd
 36+ * basically be just testing the test code. So, don't do it.
 37+ * Those should definitely be tested in the various gateway-specific test
 38+ * classes.
3539 *
3640 * @group Fundraising
3741 * @group Splunge
——@@ -51,7 +55,7 @@
5256 $gateway->publicCurrentTransaction( 'Test1' );
5357 $built = $gateway->buildRequestXML();
5458 $expected = '<?xml version="1.0"?>' . "\n";
55 - $expected .= '<XML><REQUEST><ACTION>Donate</ACTION><ACCOUNT><MERCHANTID>128</MERCHANTID><PASSWORD>k4ftw</PASSWORD><VERSION>3.2</VERSION><RETURNURL>http://' . TESTS_HOSTNAME . '/index.php/Donate-thanks/en</RETURNURL></ACCOUNT><DONATION><DONOR>Tester Testington</DONOR><AMOUNT>35000</AMOUNT><CURRENCYCODE>USD</CURRENCYCODE><LANGUAGECODE>en</LANGUAGECODE><COUNTRYCODE>US</COUNTRYCODE></DONATION></REQUEST></XML>' . "\n";
 59+ $expected .= '<XML><REQUEST><ACTION>Donate</ACTION><ACCOUNT><MERCHANTID>128</MERCHANTID><PASSWORD>k4ftw</PASSWORD><VERSION>3.2</VERSION><RETURNURL>http://' . TESTS_HOSTNAME . '/index.php/Special:GlobalCollectGatewayResult</RETURNURL></ACCOUNT><DONATION><DONOR>Tester Testington</DONOR><AMOUNT>35000</AMOUNT><CURRENCYCODE>USD</CURRENCYCODE><LANGUAGECODE>en</LANGUAGECODE><COUNTRYCODE>US</COUNTRYCODE></DONATION></REQUEST></XML>' . "\n";
5660 $this->assertEquals($built, $expected, "The constructed XML for transaction type Test1 does not match our expected.");
5761
5862 }
——@@ -320,7 +324,8 @@
321325 public function curl_transaction($data) {
322326 $data = "";
323327 $data['result'] = 'BLAH BLAH BLAH BLAH whatever something blah blah<?xml version="1.0"?>' . "\n" . '<XML><Response><Status>AOK</Status><ImportantData><thing>stuff</thing><otherthing>12</otherthing></ImportantData><errorswarnings><warning><code>128</code><message>Your shoe\'s untied...</message></warning><warning><code>45</code><message>Low clearance!</message></warning></errorswarnings></Response></XML>';
324 - return $data;
 328+ $this->setTransactionResult( $data );
 329+ return true;
325330 }
326331 }
327332
Index: branches/fundraising/extensions/DonationInterface/payflowpro_gateway/payflowpro_gateway.body.php
——@@ -142,7 +142,6 @@
143143 $this->errors['retryMsg'] = $responseMsg;
144144 $this->displayForm( $data, $this->errors );
145145 }
146 - $this->displayResultsForDebug( $result );
147146 }
148147
149148 /**
Index: branches/fundraising/extensions/DonationInterface/payflowpro_gateway/payflowpro.adapter.php
——@@ -124,10 +124,11 @@
125125
126126 /**
127127 * Interpret response code, return
128 - * 1 if approved
129 - * 2 if declined
 128+ * 1 if approved - 'complete'
 129+ * 2 if declined - 'failed'
130130 * 3 if invalid data was submitted by user
131131 * 4 all other errors
 132+ * 5 if pending - 'pending'
132133 */
133134 function getResponseErrors( $response ) {
134135
——@@ -142,36 +143,47 @@
143144 switch ( $resultCode ) {
144145 case '0':
145146 $errors['1'] = wfMsg( 'payflowpro_gateway-response-0' );
 147+ $this->setTransactionWMFStatus( 'complete' );
146148 break;
147149 case '126':
148150 $errors['5'] = wfMsg( 'payflowpro_gateway-response-126-2' );
 151+ $this->setTransactionWMFStatus( 'pending' );
149152 break;
150153 case '12':
151154 $errors['2'] = wfMsg( 'payflowpro_gateway-response-12' );
 155+ $this->setTransactionWMFStatus( 'failed' );
152156 break;
153157 case '13':
154158 $errors['2'] = wfMsg( 'payflowpro_gateway-response-13' );
 159+ $this->setTransactionWMFStatus( 'failed' );
155160 break;
156161 case '114':
157162 $errors['2'] = wfMsg( 'payflowpro_gateway-response-114' );
 163+ $this->setTransactionWMFStatus( 'failed' );
158164 break;
159165 case '4':
160166 $errors['3'] = wfMsg( 'payflowpro_gateway-response-4' );
 167+ $this->setTransactionWMFStatus( 'failed' );
161168 break;
162169 case '23':
163170 $errors['3'] = wfMsg( 'payflowpro_gateway-response-23' );
 171+ $this->setTransactionWMFStatus( 'failed' );
164172 break;
165173 case '24':
166174 $errors['3'] = wfMsg( 'payflowpro_gateway-response-24' );
 175+ $this->setTransactionWMFStatus( 'failed' );
167176 break;
168177 case '112':
169178 $errors['3'] = wfMsg( 'payflowpro_gateway-response-112' );
 179+ $this->setTransactionWMFStatus( 'failed' );
170180 break;
171181 case '125':
172182 $errors['3'] = wfMsg( 'payflowpro_gateway-response-125-2' );
 183+ $this->setTransactionWMFStatus( 'failed' );
173184 break;
174185 default:
175186 $errors['4'] = wfMsg( 'payflowpro_gateway-response-default' );
 187+ $this->setTransactionWMFStatus( 'failed' );
176188 }
177189
178190 return $errors;
——@@ -182,6 +194,7 @@
183195 * return a key/value array
184196 */
185197 function getResponseData( $response ) {
 198+
186199 if ( is_array( $response ) && !empty( $response ) ) {
187200 return $response;
188201 }
Index: branches/fundraising/extensions/DonationInterface/globalcollect_gateway/globalcollect_resultswitcher.body.php
——@@ -76,7 +76,7 @@
7777 $this->displayResultsForDebug( $result );
7878 //do the switching between the... stuff.
7979
80 - switch ( $result['data']['WMF_STATUS'] ) {
 80+ switch ( $this->adapter->getTransactionWMFStatus() ) {
8181 case 'complete':
8282 case 'pending':
8383 case 'pending-poke':
Index: branches/fundraising/extensions/DonationInterface/globalcollect_gateway/globalcollect.adapter.php
——@@ -281,7 +281,7 @@
282282 break;
283283 case 'GET_ORDERSTATUS':
284284 $data = $this->xmlChildrenToArray( $response, 'STATUS' );
285 - $data['WMF_STATUS'] = $this->findCodeAction( 'GET_ORDERSTATUS', 'STATUSID', $data['STATUSID'] );
 285+ $this->setTransactionWMFStatus( $this->findCodeAction( 'GET_ORDERSTATUS', 'STATUSID', $data['STATUSID'] ) );
286286 $data['ORDER'] = $this->xmlChildrenToArray( $response, 'ORDER' );
287287 break;
288288 }
Index: branches/fundraising/extensions/DonationInterface/gateway_common/gateway.adapter.php
——@@ -461,6 +461,11 @@
462462
463463 // log that the transaction is essentially complete
464464 self::log( $this->getData( 'order_id' ) . " Transaction complete." );
 465+
 466+ //if we're not actively adding the donor data to the session, kill it.
 467+ if ( !$this->transaction_option( 'addDonorDataToSession' ) ) {
 468+ $this->unsetAllGatewaySessionData();
 469+ }
465470
466471 return $this->getTransactionAllResults();
467472
——@@ -974,9 +979,8 @@
975980 * @return mixed WMF Transaction results status, or false if not set.
976981 */
977982 public function getTransactionWMFStatus() {
978 - if ( array_key_exists( 'data', $this->transaction_results ) &&
979 - array_key_exists( 'WMF_STATUS', $this->transaction_results['data'] ) ) {
980 - return $this->transaction_results['data']['WMF_STATUS'];
 983+ if ( array_key_exists( 'WMF_STATUS', $this->transaction_results ) ) {
 984+ return $this->transaction_results['WMF_STATUS'];
981985 } else {
982986 return false;
983987 }
——@@ -987,7 +991,7 @@
988992 * switching on behavior.
989993 */
990994 public function setTransactionWMFStatus( $status ) {
991 - $this->transaction_results['data']['WMF_STATUS'] = $status;
 995+ $this->transaction_results['WMF_STATUS'] = $status;
992996 }
993997
994998 public function getTransactionMessage() {
Index: branches/fundraising/extensions/DonationInterface/gateway_common/DonationData.php
——@@ -401,7 +401,9 @@
402402 } else {
403403 $gateway_ident = "DonationData";
404404 }
405 - unset( $_SESSION[$gateway_ident . 'EditToken'] );
 405+ if ( isset( $_SESSION ) && isset( $_SESSION[$gateway_ident . 'EditToken'] ) ){
 406+ unset( $_SESSION[$gateway_ident . 'EditToken'] );
 407+ }
406408 }
407409
408410 /**

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r98498Rebuilding the payflowpro gateway and adapter, Part 2 of Several!...khorn01:45, 30 September 2011

Status & tagging log