jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/350959 )
Change subject: processDonorReturn returns PaymentResult
......................................................................
processDonorReturn returns PaymentResult
More functional, allows more options than just success or failure.
Add tests around processDonorReturn
(Paypal express tests come in the next patch)
Bug: T163458
Change-Id: I423c77c983e873d556c708c085cbffecf545068b
---
M adyen_gateway/adyen.adapter.php
M astropay_gateway/astropay.adapter.php
M gateway_common/GatewayPage.php
M gateway_common/GatewayType.php
M gateway_common/PaymentResult.php
M gateway_common/gateway.adapter.php
M globalcollect_gateway/globalcollect.adapter.php
M paypal_gateway/express_checkout/paypal_express.adapter.php
M tests/phpunit/Adapter/Adyen/AdyenTest.php
M tests/phpunit/Adapter/AstroPay/AstroPayTest.php
M tests/phpunit/Adapter/GlobalCollect/GlobalCollectTest.php
11 files changed, 124 insertions(+), 23 deletions(-)
Approvals:
XenoRyet: Looks good to me, approved
jenkins-bot: Verified
diff --git a/adyen_gateway/adyen.adapter.php b/adyen_gateway/adyen.adapter.php
index 77f4f64..b1ddad6 100644
--- a/adyen_gateway/adyen.adapter.php
+++ b/adyen_gateway/adyen.adapter.php
@@ -43,6 +43,8 @@
);
}
+ // FIXME: That's not what ReturnValueMap is for!
+ // Unused?
function defineReturnValueMap() {
$this->return_value_map = array(
'authResult' => 'result',
@@ -222,6 +224,7 @@
/**
* @param array $requestValues GET/POST params from request
* @throws ResponseProcessingException
+ * @return PaymentResult
*/
public function processDonorReturn( $requestValues ) {
// Always called outside do_transaction, so just make a new
response object
@@ -254,6 +257,7 @@
$this->transaction_response->setGatewayTransactionId(
$gateway_txn_id );
$result_code = isset( $requestValues['authResult'] ) ?
$requestValues['authResult'] : '';
+ $paymentResult = null;
if ( $result_code == 'PENDING' || $result_code == 'AUTHORISED'
) {
// Both of these are listed as pending because we have
to submit a capture
// request on 'AUTHORIZATION' ipn message receipt.
@@ -266,19 +270,23 @@
if ( $action === 'process' ) {
$this->logger->info( "User came back as pending
or authorised, placing in payments-init queue" );
$this->finalizeInternalStatus(
FinalStatus::PENDING );
+ $paymentResult = PaymentResult::newSuccess();
} else {
$this->logger->info(
"User came back authorized but with
action $action. " .
"Showing a fail page, but leaving
details in case of manual capture."
);
$this->finalizeInternalStatus(
FinalStatus::FAILED );
+ $paymentResult = PaymentResult::newFailure();
}
}
else {
$this->finalizeInternalStatus( FinalStatus::FAILED );
+ $paymentResult = PaymentResult::newFailure();
$this->logger->info( "Negative response from gateway.
Full response: " . print_r( $requestValues, TRUE ) );
}
$this->postProcessDonation();
+ return $paymentResult;
}
/**
diff --git a/astropay_gateway/astropay.adapter.php
b/astropay_gateway/astropay.adapter.php
index 3788d93..581b679 100644
--- a/astropay_gateway/astropay.adapter.php
+++ b/astropay_gateway/astropay.adapter.php
@@ -299,6 +299,10 @@
$this->logger->info( "Payment status $status coming back to
ResultSwitcher" );
$this->finalizeInternalStatus( $status );
$this->postProcessDonation();
+ return PaymentResult::fromResults(
+ $this->transaction_response,
+ $status
+ );
}
/**
diff --git a/gateway_common/GatewayPage.php b/gateway_common/GatewayPage.php
index a41cd23..eb466de 100644
--- a/gateway_common/GatewayPage.php
+++ b/gateway_common/GatewayPage.php
@@ -429,17 +429,9 @@
if ( $this->adapter->checkTokens() ) {
// feed processDonorReturn all the GET and POST vars
$requestValues = $this->getRequest()->getValues();
- $this->adapter->processDonorReturn( $requestValues );
- $status = $this->adapter->getFinalStatus();
-
- switch ( $status ) {
- case FinalStatus::COMPLETE:
- case FinalStatus::PENDING:
- case FinalStatus::PENDING_POKE:
- $this->displayThankYouPage( $status );
- return;
- }
- $this->logger->info( "Displaying fail page for final
status $status" );
+ $result = $this->adapter->processDonorReturn(
$requestValues );
+ $this->renderResponse( $result );
+ return;
} else {
$this->logger->error( "Resultswitcher: Token Check
Failed. Order ID: $oid" );
}
diff --git a/gateway_common/GatewayType.php b/gateway_common/GatewayType.php
index 3d42622..8389b1c 100644
--- a/gateway_common/GatewayType.php
+++ b/gateway_common/GatewayType.php
@@ -51,6 +51,7 @@
* Perform any additional processing required when donor returns from
* payment processor site. Should set the final status.
* @param array $requestValues all GET and POST values from the request
+ * @return PaymentResult
*/
public function processDonorReturn( $requestValues );
diff --git a/gateway_common/PaymentResult.php b/gateway_common/PaymentResult.php
index 77acb8f..44a0970 100644
--- a/gateway_common/PaymentResult.php
+++ b/gateway_common/PaymentResult.php
@@ -27,9 +27,9 @@
protected $iframe;
protected $form;
protected $redirect;
- protected $refresh;
+ protected $refresh = false;
protected $errors = array();
- protected $failed;
+ protected $failed = false;
protected function __construct() {
}
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 62e3922..85fc888 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -1474,9 +1474,11 @@
/**
* Default implementation sets status to complete.
* @param array $requestValues all GET and POST values from the request
+ * @return PaymentResult
*/
public function processDonorReturn( $requestValues ) {
$this->finalizeInternalStatus( FinalStatus::COMPLETE );
+ return PaymentResult::newSuccess();
}
/**
diff --git a/globalcollect_gateway/globalcollect.adapter.php
b/globalcollect_gateway/globalcollect.adapter.php
index e749c48..6862e5d 100644
--- a/globalcollect_gateway/globalcollect.adapter.php
+++ b/globalcollect_gateway/globalcollect.adapter.php
@@ -1278,13 +1278,13 @@
if (! $oid) {
$this->finalizeInternalStatus( FinalStatus::FAILED );
$this->logger->error( 'Missing Order ID' );
- return;
+ return PaymentResult::newFailure();
}
if ( $this->getData_Unstaged_Escaped( 'payment_method' ) !==
'cc' ) {
$this->finalizeInternalStatus( FinalStatus::FAILED );
$this->logger->error( "Payment method is not CC, OID:
{$oid}" );
- return;
+ return PaymentResult::newFailure();
}
$session_oid = $this->session_getData( 'Donor', 'order_id' );
@@ -1297,17 +1297,21 @@
// but leave the payment to be resolved by the orphan
rectifier.
// FIXME: should use finalizeInternalStatus() but there
are side effects.
$this->final_status = FinalStatus::PENDING;
- return;
+ return PaymentResult::newSuccess();
}
if ( $oid !== $session_oid ) {
$this->logger->info( "Order ID mismatch
'{$oid}'/'{$session_oid}'" );
// FIXME: should use finalizeInternalStatus() but there
are side effects
$this->final_status = FinalStatus::PENDING;
- return;
+ return PaymentResult::newSuccess();
}
- $this->do_transaction( 'Confirm_CreditCard' );
+ $response = $this->do_transaction( 'Confirm_CreditCard' );
+ return PaymentResult::fromResults(
+ $response,
+ $this->getFinalStatus()
+ );
}
/**
diff --git a/paypal_gateway/express_checkout/paypal_express.adapter.php
b/paypal_gateway/express_checkout/paypal_express.adapter.php
index 90a1671..4fee2d4 100644
--- a/paypal_gateway/express_checkout/paypal_express.adapter.php
+++ b/paypal_gateway/express_checkout/paypal_express.adapter.php
@@ -489,7 +489,7 @@
$resultData = $this->do_transaction( 'DoExpressCheckoutPayment'
);
if ( !$resultData->getCommunicationStatus() ) {
$this->finalizeInternalStatus( FinalStatus::FAILED );
- return;
+ return PaymentResult::newFailure();
}
if ( $this->getData_Unstaged_Escaped( 'recurring' ) ) {
@@ -504,6 +504,10 @@
'Failed to create a recurring profile',
ResponseCodes::UNKNOWN );
}
}
+ return PaymentResult::fromResults(
+ $this->getTransactionResponse(),
+ $this->getFinalStatus()
+ );
}
/**
diff --git a/tests/phpunit/Adapter/Adyen/AdyenTest.php
b/tests/phpunit/Adapter/Adyen/AdyenTest.php
index ce8a7e4..9f8a499 100644
--- a/tests/phpunit/Adapter/Adyen/AdyenTest.php
+++ b/tests/phpunit/Adapter/Adyen/AdyenTest.php
@@ -154,4 +154,50 @@
$this->assertEquals( $expected, $ret, 'Adyen "donate"
transaction not constructing the expected redirect URL' );
$this->assertNotNull( $gateway->getData_Unstaged_Escaped(
'order_id' ), "Adyen order_id is null, and we need one for 'merchantReference'"
);
}
+
+ public function testDonorReturnSuccess() {
+ $init = $this->getDonorTestData();
+ $init['payment_method'] = 'cc';
+ $init['payment_submethod'] = 'visa';
+ $init['language'] = 'FR';
+ $init['order_id'] = '55555';
+ $session['Donor'] = $init;
+ $this->setUpRequest( $init, $session );
+ $gateway = $this->getFreshGatewayObject( array() );
+ $result = $gateway->processDonorReturn( array(
+ 'authResult' => 'AUTHORISED',
+ 'merchantReference' => '55555.0',
+ 'merchantSig' =>
'o1QTd6X/PYrOgLPoSheamR3osAksh6oTaSytsCcJsFA=',
+ 'paymentMethod' => 'visa',
+ 'pspReference' => '123987612346789',
+ 'shopperLocale' => 'fr_FR',
+ 'skinCode' => 'testskin',
+ 'title' => 'Special:AdyenGatewayResult'
+ ) );
+ $this->assertFalse( $result->isFailed() );
+ $this->assertEmpty( $result->getErrors() );
+ // TODO inspect the queue message
+ }
+
+ public function testDonorReturnFailure() {
+ $init = $this->getDonorTestData();
+ $init['payment_method'] = 'cc';
+ $init['payment_submethod'] = 'visa';
+ $init['language'] = 'FR';
+ $init['order_id'] = '55555';
+ $session['Donor'] = $init;
+ $this->setUpRequest( $init, $session );
+ $gateway = $this->getFreshGatewayObject( array() );
+ $result = $gateway->processDonorReturn( array(
+ 'authResult' => 'REFUSED',
+ 'merchantReference' => '55555.0',
+ 'merchantSig' =>
'EVqAiz4nZ8XQ9Wfbm9bOQYaKPV22qdY+/6va7zAo580=',
+ 'paymentMethod' => 'visa',
+ 'pspReference' => '123987612346789',
+ 'shopperLocale' => 'fr_FR',
+ 'skinCode' => 'testskin',
+ 'title' => 'Special:AdyenGatewayResult'
+ ) );
+ $this->assertTrue( $result->isFailed() );
+ }
}
diff --git a/tests/phpunit/Adapter/AstroPay/AstroPayTest.php
b/tests/phpunit/Adapter/AstroPay/AstroPayTest.php
index 90a7dbc..33a85e0 100644
--- a/tests/phpunit/Adapter/AstroPay/AstroPayTest.php
+++ b/tests/phpunit/Adapter/AstroPay/AstroPayTest.php
@@ -363,7 +363,8 @@
'x_invoice' => '123456789',
);
- $gateway->processDonorReturn( $requestValues );
+ $result = $gateway->processDonorReturn( $requestValues );
+ $this->assertFalse( $result->isFailed() );
$status = $gateway->getFinalStatus();
$this->assertEquals( FinalStatus::COMPLETE, $status );
}
@@ -393,7 +394,8 @@
'x_invoice' => '123456789',
);
- $gateway->processDonorReturn( $requestValues );
+ $result = $gateway->processDonorReturn( $requestValues );
+ $this->assertFalse( $result->isFailed() );
$amount = $gateway->getData_Unstaged_Escaped( 'amount' );
$this->assertEquals( '100.00', $amount, 'Not recording correct
amount' );
}
@@ -418,7 +420,8 @@
'x_invoice' => '123456789',
);
- $gateway->processDonorReturn( $requestValues );
+ $result = $gateway->processDonorReturn( $requestValues );
+ $this->assertTrue( $result->isFailed() );
$status = $gateway->getFinalStatus();
$this->assertEquals( FinalStatus::FAILED, $status );
}
diff --git a/tests/phpunit/Adapter/GlobalCollect/GlobalCollectTest.php
b/tests/phpunit/Adapter/GlobalCollect/GlobalCollectTest.php
index 869d1e4..52abfbe 100644
--- a/tests/phpunit/Adapter/GlobalCollect/GlobalCollectTest.php
+++ b/tests/phpunit/Adapter/GlobalCollect/GlobalCollectTest.php
@@ -583,7 +583,7 @@
/**
* doPayment should recover from Ingenico-side timeouts.
*/
- function testTimoutRecover() {
+ function testTimeoutRecover() {
$init = $this->getDonorTestData();
$init['payment_method'] = 'cc';
$init['payment_submethod'] = 'visa';
@@ -596,4 +596,41 @@
$loglines = $this->getLogMatches( LogLevel::INFO, '/Repeating
transaction for timeout/' );
$this->assertNotEmpty( $loglines, "Log does not say we retried
for timeout." );
}
+
+ public function testDonorReturnSuccess() {
+ $init = $this->getDonorTestData( 'FR' );
+ $init['payment_method'] = 'cc';
+ $init['payment_submethod'] = 'visa';
+ $init['email'] = '[email protected]';
+ $init['order_id'] = mt_rand();
+ $session['Donor'] = $init;
+ $this->setUpRequest( $init, $session );
+ $gateway = $this->getFreshGatewayObject( array() );
+ $result = $gateway->processDonorReturn( array(
+ 'REF' => $init['order_id'],
+ 'CVVRESULT' => 'M',
+ 'AVSRESULT' => '0'
+ ) );
+ $this->assertFalse( $result->isFailed() );
+ $this->assertEmpty( $result->getErrors() );
+ // TODO inspect the queue message
+ }
+
+ public function testDonorReturnFailure() {
+ $init = $this->getDonorTestData();
+ $init['payment_method'] = 'cc';
+ $init['payment_submethod'] = 'visa';
+ $init['email'] = '[email protected]';
+ $init['order_id'] = mt_rand();
+ $session['Donor'] = $init;
+ $this->setUpRequest( $init, $session );
+ $gateway = $this->getFreshGatewayObject( array() );
+ $gateway->setDummyGatewayResponseCode( '430285' ); // invalid
card
+ $result = $gateway->processDonorReturn( array(
+ 'REF' => $init['order_id'],
+ 'CVVRESULT' => 'M',
+ 'AVSRESULT' => '0'
+ ) );
+ $this->assertTrue( $result->isFailed() );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/350959
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I423c77c983e873d556c708c085cbffecf545068b
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <[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
