r101576 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r101575‎ | r101576 | r101577 >
Date:02:22, 2 November 2011
Author:kaldari
Status:resolved (Comments)
Tags:
Comment:
begining to handle transaction errors for globalcollect - this might be all wrong!
Modified paths:
  • /trunk/extensions/DonationInterface/globalcollect_gateway/forms/html/webitects_2_3step.html (modified) (history)
  • /trunk/extensions/DonationInterface/globalcollect_gateway/globalcollect_gateway.body.php (modified) (history)
  • /trunk/extensions/DonationInterface/globalcollect_gateway/globalcollect_resultswitcher.body.php (modified) (history)
  • /trunk/extensions/DonationInterface/payflowpro_gateway/payflowpro_gateway.body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/DonationInterface/payflowpro_gateway/payflowpro_gateway.body.php
——@@ -126,7 +126,7 @@
127127 * Display response message to user with submitted user-supplied data
128128 *
129129 * @param $data Array: array of posted data from form
130 - * @param $responseMsg String: message supplied by getResults function
 130+ * @param $responseMsg String: message supplied by fnPayflowDisplayResults function
131131 */
132132 function fnPayflowDisplayApprovedResults( $data, $responseMsg ) {
133133 global $wgOut;
——@@ -158,7 +158,7 @@
159159 /**
160160 * Display response message to user with submitted user-supplied data
161161 *
162 - * @param $responseMsg String: message supplied by getResults function
 162+ * @param $responseMsg String: message supplied by fnPayflowDisplayResults function
163163 */
164164 function fnPayflowDisplayDeclinedResults( $responseMsg ) {
165165 global $wgOut;
——@@ -178,7 +178,7 @@
179179 /**
180180 * Display response message when there is a system error unrelated to user's entry
181181 *
182 - * @param $responseMsg String: message supplied by getResults function
 182+ * @param $responseMsg String: message supplied by fnPayflowDisplayResults function
183183 */
184184 function fnPayflowDisplayOtherResults( $responseMsg ) {
185185 //I have collapsed it like this because the contents were identical.
Index: trunk/extensions/DonationInterface/globalcollect_gateway/globalcollect_resultswitcher.body.php
——@@ -3,7 +3,7 @@
44 class GlobalCollectGatewayResult extends GatewayForm {
55
66 /**
7 - * Defines the action to take on a PFP transaction.
 7+ * Defines the action to take on a GlobalCollect transaction.
88 *
99 * Possible values include 'process', 'challenge',
1010 * 'review', 'reject'. These values can be set during
——@@ -82,7 +82,7 @@
8383 $go = $this->adapter->getThankYouPage();
8484 break;
8585 case 'failed':
86 - $go = $this->adapter->getFailPage();
 86+ $go = $this->getDeclinedResultPage();
8787 break;
8888 }
8989
——@@ -96,6 +96,33 @@
9797 }
9898 }
9999 }
 100+
 101+ /**
 102+ * Get the URL to redirect to when the transaction has been declined. This will be the form the
 103+ * user came from with all the data and an error message.
 104+ */
 105+ function getDeclinedResultPage() {
 106+ global $wgOut;
 107+ $failpage = $this->adapter->getGlobal( 'FailPage' );
 108+
 109+ if ( $failpage ) {
 110+ $wgOut->redirect( $failpage . "/" . $data['language'] );
 111+ } else {
 112+ // general decline message
 113+ $declinedDefault = wfMsg( 'php-response-declined' );
 114+
 115+ $data = $this->adapter->getData();
 116+ $referrer = $data['referrer'];
 117+ unset( $data['referrer'] );
 118+ $data['amount'] = $data['amount']/100;
 119+ $data['error'] = $declinedDefault;
 120+
 121+ $params = wfArrayToCGI( $data );
 122+ $returnto = htmlspecialchars_decode( $referrer ) . '&' . $params;
 123+ return $returnto;
 124+ }
 125+ }
 126+
100127 }
101128
102129 // end class
Index: trunk/extensions/DonationInterface/globalcollect_gateway/forms/html/webitects_2_3step.html
——@@ -58,7 +58,7 @@
5959 <input class="btn" id="pp" type="button" value="%donate_interface-paypal-button%"/><span id='loading'></span>
6060 </p>
6161 </div>
62 - <div id="step2header"><h3>%donate_interface-billing-address% <span class="mute" id="change-billing" style="display: none;">(<a href="#">%donation_interface-change%</a>)</span></h3></div>
 62+ <div id="step2header"><h3>%donate_interface-billing-address% <span class="mute" id="change-billing" style="display: none;">(<a href="#">%donate_interface-change%</a>)</span></h3></div>
6363 <div id="step2wrapper">
6464 <div id="billing-content" class="gainlayout">
6565 <div id="billingErrorMessages" class="small"></div>
Index: trunk/extensions/DonationInterface/globalcollect_gateway/globalcollect_gateway.body.php
——@@ -162,7 +162,7 @@
163163 // Display form for the first time
164164 $oid = $wgRequest->getText( 'order_id' );
165165 if ( $oid ) {
166 - $wgOut->addHTML( "<pre>CAME BACK FROM SOMETHING.</pre>" );
 166+ // $wgOut->addHTML( "<pre>CAME BACK FROM SOMETHING.</pre>" );
167167 $result = $this->adapter->do_transaction( 'GET_ORDERSTATUS' );
168168 $this->displayResultsForDebug( $result );
169169 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r101764follow-up to r101576 to address comments. Next I will be experimenting with p...kaldari00:34, 3 November 2011
r101768MFT r101576, r101764awjrichards00:41, 3 November 2011
r101781follow-up to r101764 and r101576, fixing bogus call to $datakaldari02:43, 3 November 2011
r102044The redirection actually takes place just fine on the outside of this functio...khorn20:40, 4 November 2011

Comments

#Comment by Awjrichards (talk | contribs)   04:11, 2 November 2011

Out of curiosity: 

$data = $this->adapter->getData();
+			$referrer = $data['referrer'];
+			unset( $data['referrer'] );
+			$data['amount'] = $data['amount']/100;
+			$data['error'] = $declinedDefault;
+			
+			$params = wfArrayToCGI( $data );
+			$returnto = htmlspecialchars_decode( $referrer ) . '&' . $params;
+			return $returnto;

Why not: 

$data = $this->adapter->getData();
			$data['referrer'] = htmlspecialchars_decode( $referrer );
			$data['amount'] = $data['amount']/100;
			$data['error'] = $declinedDefault;
			
			return wfArrayToCGI( $data );

Also (either way) - this just returns a query string. Is this expected? Does $wgOut->redirect() handle just a query string in the way you're expecting it to here?

#Comment by Awjrichards (talk | contribs)   00:04, 3 November 2011

We just chatted IRL and it turns out it's actually written almost correctly. The $referrer is actually the URL the user is being returned to and $params gets tacked on to the query string. This makes more sense then :p

Please make that clear in a comment. Also, we should make sure that $referrer already has a query string, otherwise doing: 

$returnto = htmlspecialchars_decode( $referrer ) . '&' . $params;

will have icky results.

Status & tagging log