jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/359213 )
Change subject: Send resultswitcher reloaders to TY page
......................................................................
Send resultswitcher reloaders to TY page
This restores some logic we used to have that kept track of processed
orders in a key outside of the donor data.
TODO: shouldn't globalcollectadapter return true for
isReturnProcessingRequired() ?
Bug: T167990
Change-Id: I9d23953e180b85d0d7ae357442760dd5c4d4619c
---
M gateway_common/GatewayPage.php
M gateway_common/gateway.adapter.php
M paypal_gateway/express_checkout/paypal_express.adapter.php
M tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
M tests/phpunit/includes/test_gateway/TestingPaypalExpressAdapter.php
5 files changed, 136 insertions(+), 0 deletions(-)
Approvals:
Mepps: Looks good to me, approved
jenkins-bot: Verified
diff --git a/gateway_common/GatewayPage.php b/gateway_common/GatewayPage.php
index 4b1f2ba..1d0d271 100644
--- a/gateway_common/GatewayPage.php
+++ b/gateway_common/GatewayPage.php
@@ -398,6 +398,17 @@
$this->setHeaders();
+ if ( $this->isRepeatReturnProcess() ) {
+ $this->logger->warning(
+ 'Donor is trying to process an
already-processed payment. ' .
+ "Adapter Order ID: $oid.\n" .
+ "Cookies: " . print_r( $_COOKIE, true ) ."\n" .
+ "User-Agent: " . $_SERVER['HTTP_USER_AGENT']
+ );
+ $this->displayThankYouPage( 'repeat return processing'
);
+ return;
+ }
+
if ( $deadSession ){
if ( $this->adapter->isReturnProcessingRequired() ) {
wfHttpError( 403, 'Forbidden', wfMessage(
'donate_interface-error-http-403' )->text() );
@@ -429,6 +440,7 @@
// feed processDonorReturn all the GET and POST vars
$requestValues = $this->getRequest()->getValues();
$result = $this->adapter->processDonorReturn(
$requestValues );
+ $this->markReturnProcessed();
$this->renderResponse( $result );
return;
} else {
@@ -567,4 +579,34 @@
);
}
}
+
+ protected function isRepeatReturnProcess() {
+ $request = $this->getRequest();
+ $requestProcessId = $this->adapter->getRequestProcessId(
+ $request->getValues()
+ );
+ $processedRequests = $request->getSessionData(
'processed_requests' );
+ if ( !$requestProcessId || empty( $processedRequests ) ) {
+ return false;
+ }
+ return array_key_exists( $requestProcessId, $processedRequests
);
+ }
+
+ protected function markReturnProcessed() {
+ $request = $this->getRequest();
+ $requestProcessId = $this->adapter->getRequestProcessId(
+ $request->getValues()
+ );
+ if ( !$requestProcessId ) {
+ return;
+ }
+ $processedRequests = $request->getSessionData(
'processed_requests' );
+ if ( !$processedRequests ) {
+ $processedRequests = array();
+ }
+ // TODO: we could store the results of the last process here,
but for now
+ // we just indicate we did SOMETHING with it
+ $processedRequests[$requestProcessId] = true;
+ $request->setSessionData( 'processed_requests',
$processedRequests );
+ }
}
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 9e15ed5..be079a7 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -1453,6 +1453,18 @@
}
/**
+ * Gateways which return true from isReturnProcessingRequired must
+ * override this with logic to get an ID from the request which will
+ * identify repeated attempts to process the same payment.
+ *
+ * @param array $requestValues
+ * @return int|string Order id
+ */
+ protected function getRequestProcessId( $requestValues ) {
+ return null;
+ }
+
+ /**
* Process the API response obtained from the payment processor and set
* properties of transaction_response.
* Default implementation just says we got a response.
@@ -2953,6 +2965,7 @@
'PaymentForms',
'numAttempt',
'order_status', //for post-payment activities
+ 'processed_requests', //for post-payment
activities
'sequence',
);
$preservedData = array();
diff --git a/paypal_gateway/express_checkout/paypal_express.adapter.php
b/paypal_gateway/express_checkout/paypal_express.adapter.php
index 77f5b9f..c614407 100644
--- a/paypal_gateway/express_checkout/paypal_express.adapter.php
+++ b/paypal_gateway/express_checkout/paypal_express.adapter.php
@@ -388,6 +388,10 @@
return true;
}
+ public function getRequestProcessId( $requestValues ) {
+ return $requestValues['token'];
+ }
+
protected function processResponse( $response ) {
$this->transaction_response->setData( $response );
// FIXME: I'm not sure why we're responsible for failing the
diff --git a/tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
b/tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
index a57ce09..c949fa5 100644
--- a/tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
+++ b/tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
@@ -95,6 +95,7 @@
function testProcessDonorReturn() {
$init = $this->getDonorTestData( 'US' );
$init['contribution_tracking_id'] = '45931210';
+ $this->setUpRequest( $init, array( 'Donor' => $init ) );
$gateway = $this->getFreshGatewayObject( $init );
$gateway->setDummyGatewayResponseCode( 'OK' );
@@ -197,6 +198,7 @@
function testProcessDonorReturnPaymentRetry() {
$init = $this->getDonorTestData( 'US' );
$init['contribution_tracking_id'] = '45931210';
+ $this->setUpRequest( $init, array( 'Donor' => $init ) );
$gateway = $this->getFreshGatewayObject( $init );
$gateway->setDummyGatewayResponseCode( '10486' );
@@ -239,4 +241,78 @@
);
}
+ /**
+ * The result switcher should redirect the donor to the thank you page
and mark the token as
+ * processed.
+ */
+ public function testResultSwitcher() {
+ $init = $this->getDonorTestData( 'US' );
+ $init['contribution_tracking_id'] = '45931210';
+ $init['gateway_session_id'] = mt_rand();
+ $session = array( 'Donor' => $init );
+
+ $request = array(
+ 'token' => $init['gateway_session_id'],
+ 'PayerID' => 'ASdASDAS',
+ 'language' => 'pt' // FIXME: mashing up request vars
and other stuff in verifyFormOutput
+ );
+ $assertNodes = array(
+ 'headers' => array(
+ 'Location' => function( $location ) use ( $init
) {
+ // Do this after the real processing to
avoid side effects
+ $gateway =
$this->getFreshGatewayObject( $init );
+ $url = ResultPages::getThankYouPage(
$gateway );
+ return $location === $url;
+ }
+ )
+ );
+
+ $this->verifyFormOutput( 'PaypalExpressGatewayResult',
$request, $assertNodes, false, $session );
+ $processed =
RequestContext::getMain()->getRequest()->getSessionData( 'processed_requests' );
+ $this->assertArrayHasKey( $request['token'], $processed );
+
+ // Make sure we logged the expected cURL attempts
+ $messages = $this->getLogMatches( 'info', '/Preparing to send
GetExpressCheckoutDetails transaction to Paypal Express Checkout/' );
+ $this->assertNotEmpty( $messages );
+ $messages = $this->getLogMatches( 'info', '/Preparing to send
DoExpressCheckoutPayment transaction to Paypal Express Checkout/' );
+ $this->assertNotEmpty( $messages );
+ }
+
+ /**
+ * The result switcher should redirect the donor to the thank you page
without
+ * re-processing the donation.
+ */
+ public function testResultSwitcherRepeat() {
+ $init = $this->getDonorTestData( 'US' );
+ $init['contribution_tracking_id'] = '45931210';
+ $init['gateway_session_id'] = mt_rand();
+ $session = array(
+ 'Donor' => $init,
+ 'processed_requests' => array(
+ $init['gateway_session_id'] => true
+ )
+ );
+
+ $request = array(
+ 'token' => $init['gateway_session_id'],
+ 'PayerID' => 'ASdASDAS',
+ 'language' => 'pt' // FIXME: mashing up request vars
and other stuff in verifyFormOutput
+ );
+ $assertNodes = array(
+ 'headers' => array(
+ 'Location' => function( $location ) use ( $init
) {
+ // Do this after the real processing to
avoid side effects
+ $gateway =
$this->getFreshGatewayObject( $init );
+ $url = ResultPages::getThankYouPage(
$gateway );
+ return $location === $url;
+ }
+ )
+ );
+
+ $this->verifyFormOutput( 'PaypalExpressGatewayResult',
$request, $assertNodes, false, $session );
+
+ // We should not have logged any cURL attempts
+ $messages = $this->getLogMatches( 'info', '/Preparing to send
.*/' );
+ $this->assertEmpty( $messages );
+ }
}
diff --git
a/tests/phpunit/includes/test_gateway/TestingPaypalExpressAdapter.php
b/tests/phpunit/includes/test_gateway/TestingPaypalExpressAdapter.php
index 5a18329..18b3f4b 100644
--- a/tests/phpunit/includes/test_gateway/TestingPaypalExpressAdapter.php
+++ b/tests/phpunit/includes/test_gateway/TestingPaypalExpressAdapter.php
@@ -4,6 +4,7 @@
* FIXME so much: DRY
*/
class TestingPaypalExpressAdapter extends PaypalExpressAdapter {
+ protected $dummyGatewayResponseCode = 'OK';
/**
* Set the error code you want the dummy response to return
--
To view, visit https://gerrit.wikimedia.org/r/359213
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9d23953e180b85d0d7ae357442760dd5c4d4619c
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <[email protected]>
Gerrit-Reviewer: AndyRussG <[email protected]>
Gerrit-Reviewer: Awight <[email protected]>
Gerrit-Reviewer: Cdentinger <[email protected]>
Gerrit-Reviewer: Eileen <[email protected]>
Gerrit-Reviewer: Katie Horn <[email protected]>
Gerrit-Reviewer: Mepps <[email protected]>
Gerrit-Reviewer: XenoRyet <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
