Ejegg has uploaded a new change for review.
https://gerrit.wikimedia.org/r/206327
Change subject: Fix comm status and error checking for PaymentStatus
......................................................................
Fix comm status and error checking for PaymentStatus
Check for the correct keys and codes in getResponseErrors
Communication status should still be true if response is valid
but indicates something else is wrong. It should only be false
when the response is malformed or missing.
Bug: T90504
Change-Id: I02dc5496fd1527ffa1cb9d2e56b9e6e71fec7587
---
M astropay_gateway/astropay.adapter.php
M tests/Adapter/Astropay/AstropayTest.php
A tests/includes/Responses/astropay/NewInvoice_notJson.testresponse
3 files changed, 85 insertions(+), 24 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface
refs/changes/27/206327/1
diff --git a/astropay_gateway/astropay.adapter.php
b/astropay_gateway/astropay.adapter.php
index fe3c070..0b06902 100644
--- a/astropay_gateway/astropay.adapter.php
+++ b/astropay_gateway/astropay.adapter.php
@@ -399,38 +399,67 @@
}
function getResponseStatus( $response ) {
- if ( $response === NULL || !isset( $response['status'] ) ) {
+ if ( $response === NULL ) {
return false;
}
- return $response['status'] === '0';
+ $valid = false;
+ switch( $this->getCurrentTransaction() ) {
+ case 'NewInvoice':
+ $valid = isset( $response['status'] );
+ break;
+ case 'PaymentStatus':
+ $valid = isset( $response['result'] );
+ break;
+ }
+ return $valid;
}
+ // TODO: getResponseErrors should just return the error codes and any
debug
+ // info, and another function should decide what to display and
translate it
function getResponseErrors( $response ) {
$logged = false;
$errors = array();
$code = 'internal-0000';
if ( $response === NULL ) {
- $logged = 'Astropay response was not valid JSON. Full
response: ' .
+ $logged = 'Astropay response was not valid. Full
response: ' .
$this->getTransactionRawResponse();
$this->logger->error( $logged );
- } else if ( !isset( $response['status'] ) ) {
- $logged = 'Astropay response does not have a status
code. Full response: ' .
- $this->getTransactionRawResponse();
- $this->logger->error( $logged );
- } else if ( $response['status'] !== '0' ) {
- $logged = "Astropay response has non-zero status
{$response['status']}. ";
- if ( isset( $response['desc'] ) ) {
- // They don't give us codes to distinguish
failure modes, so we
- // have to parse the description.
- if ( preg_match( '/invoice already used/i',
$response['desc'] ) ) {
- $code = $this::DUPLICATE_ORDER_ID_ERROR;
- }
- $logged .= 'Error description: ' .
$response['desc'];
- } else {
- $logged .= 'Full response: ' .
$this->getTransactionRawResponse();
+ } else {
+ switch( $this->getCurrentTransaction() ) {
+ case 'NewInvoice':
+ if ( !isset( $response['status'] ) ) {
+ $logged = 'Astropay response
does not have a status code. Full response: ' .
+
$this->getTransactionRawResponse();
+ $this->logger->error( $logged );
+ } else if ( $response['status'] !== '0'
) {
+ $logged = "Astropay response
has non-zero status {$response['status']}. ";
+ if ( isset( $response['desc'] )
) {
+ // They don't give us
codes to distinguish failure modes, so we
+ // have to parse the
description.
+ if ( preg_match(
'/invoice already used/i', $response['desc'] ) ) {
+ $code =
$this::DUPLICATE_ORDER_ID_ERROR;
+ }
+ $logged .= 'Error
description: ' . $response['desc'];
+ } else {
+ $logged .= 'Full
response: ' . $this->getTransactionRawResponse();
+ }
+ $this->logger->warning( $logged
);
+ }
+ break;
+ case 'PaymentStatus':
+ case 'ProcessReturn':
+ if ( !isset( $response['result'] ) ) {
+ $logged = 'Astropay response
does not have a result code. Full response: ' .
+
$this->getTransactionRawResponse();
+ $this->logger->error( $logged );
+ } else if ( $response['result'] === '6'
) {
+ $logged = 'Astropay reports
they cannot find the transaction for order ID ' .
+
$this->getData_Unstaged_Escaped( 'order_id' );
+ $this->logger->error( $logged );
+ }
+ break;
}
- $this->logger->warning( $logged );
}
if ( $logged ) {
diff --git a/tests/Adapter/Astropay/AstropayTest.php
b/tests/Adapter/Astropay/AstropayTest.php
index 6bf2c27..a0e66e8 100644
--- a/tests/Adapter/Astropay/AstropayTest.php
+++ b/tests/Adapter/Astropay/AstropayTest.php
@@ -112,8 +112,31 @@
}
/**
- * When Astropay sends back valid JSON with status "1", we should set
txn
- * status to false and error array to generic error and log a warning.
+ * If astropay sends back non-JSON, communication status should be false
+ */
+ function testGibberishResponse() {
+ $init = $this->getDonorTestData( 'BR' );
+ $this->setLanguage( $init['language'] );
+ $gateway = $this->getFreshGatewayObject( $init );
+ $gateway->setDummyGatewayResponseCode( 'notJson' );
+
+ $gateway->do_transaction( 'NewInvoice' );
+
+ $this->assertEquals( false, $gateway->getTransactionStatus(),
+ 'Transaction status should be false for bad format' );
+
+ $expected = array(
+ 'internal-0000' => wfMessage(
'donate_interface-processing-error')->inLanguage( $init['language'] )->text()
+ );
+ $this->assertEquals( $expected,
$gateway->getTransactionErrors(),
+ 'Wrong error for code bad response format' );
+ $logged = $this->getLogMatches( LogLevel::ERROR, '/Astropay
response was not valid. Full response:/' );
+ $this->assertNotEmpty( $logged );
+ }
+
+ /**
+ * When Astropay sends back valid JSON with status "1", we should set
+ * error array to generic error and log a warning.
*/
function testStatusErrors() {
$init = $this->getDonorTestData( 'BR' );
@@ -122,9 +145,6 @@
$gateway->setDummyGatewayResponseCode( '1' );
$gateway->do_transaction( 'NewInvoice' );
-
- $this->assertEquals( false, $gateway->getTransactionStatus(),
- 'Transaction status should be false for code "1"' );
$expected = array(
'internal-0000' => wfMessage(
'donate_interface-processing-error')->inLanguage( $init['language'] )->text()
diff --git a/tests/includes/Responses/astropay/NewInvoice_notJson.testresponse
b/tests/includes/Responses/astropay/NewInvoice_notJson.testresponse
new file mode 100644
index 0000000..5545628
--- /dev/null
+++ b/tests/includes/Responses/astropay/NewInvoice_notJson.testresponse
@@ -0,0 +1,12 @@
+HTTP/1.1 200 OK
+Server: nginx/1.7.9
+Date: Wed, 08 Apr 2015 00:19:52 GMT
+Content-Type: text/xml; charset=UTF-8
+Content-Length: 132
+Connection: keep-alive
+X-Powered-By: PHP/5.3.27
+
+<?xml version = "1.0"?>
+<XML>
+ <ERROR>Your request was so malformed I'm sending you a response in a
different format</ERROR>
+</XML>
--
To view, visit https://gerrit.wikimedia.org/r/206327
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I02dc5496fd1527ffa1cb9d2e56b9e6e71fec7587
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
