jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/364903 )
Change subject: PayPal EC recurring: initial charge is part of subscription
......................................................................
PayPal EC recurring: initial charge is part of subscription
We were originally doing a kludgey workaround that had an initial
standalone payment, then a subscription starting in a month. That
made it really hard for Donor Services to find subscriptions.
PayPal techs told us the reason for the kludge is gone for 99% of
users, and we're waiting on a list of countries where it still
turns up.
Bug: T170478
Change-Id: Ifebc76e0e501303381ff8c73000d02e36cdb796c
---
M paypal_gateway/express_checkout/config/var_map.yaml
M paypal_gateway/express_checkout/paypal_express.adapter.php
M tests/phpunit/Adapter/PayPal/PayPalApiTest.php
M tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
A
tests/phpunit/includes/Responses/paypal_ec/CreateRecurringPaymentsProfile_10486.testresponse
M
tests/phpunit/includes/Responses/paypal_ec/CreateRecurringPaymentsProfile_Recurring-OK.testresponse
A
tests/phpunit/includes/Responses/paypal_ec/SetExpressCheckout_recurring_OK.testresponse
7 files changed, 147 insertions(+), 31 deletions(-)
Approvals:
Mepps: Looks good to me, approved
jenkins-bot: Verified
diff --git a/paypal_gateway/express_checkout/config/var_map.yaml
b/paypal_gateway/express_checkout/config/var_map.yaml
index 40e9b9f..1b7032f 100644
--- a/paypal_gateway/express_checkout/config/var_map.yaml
+++ b/paypal_gateway/express_checkout/config/var_map.yaml
@@ -24,6 +24,7 @@
PROFILEID: subscr_id
PROFILEREFERENCE: contribution_tracking_id
PROFILESTARTDATE: date
+TRANSACTIONID: gateway_txn_id
# TODO: discuss whether to capture
#PHONENUM: phone
RETURNURL: returnto
diff --git a/paypal_gateway/express_checkout/paypal_express.adapter.php
b/paypal_gateway/express_checkout/paypal_express.adapter.php
index f40e39b..6eabf1b 100644
--- a/paypal_gateway/express_checkout/paypal_express.adapter.php
+++ b/paypal_gateway/express_checkout/paypal_express.adapter.php
@@ -200,7 +200,7 @@
'NOSHIPPING' => 1,
'L_BILLINGTYPE0' => 'RecurringPayments',
// FIXME: Sad! The thank-you message would be
perfect here,
- // but it seems the exlamation mark is not
supported, even when
+ // but it seems the exclamation mark is not
supported, even when
// urlencoded properly.
//'L_BILLINGAGREEMENTDESCRIPTION0' =>
WmfFramework::formatMessage(
'donate_interface-donate-error-thank-you-for-your-support' ),
'L_BILLINGAGREEMENTDESCRIPTION0' =>
WmfFramework::formatMessage( 'donate_interface-monthly-donation-description' ),
@@ -408,8 +408,16 @@
case 'CreateRecurringPaymentsProfile':
$this->checkResponseAck( $response );
- // Grab the subscription ID.
+ // Grab the subscription ID and transaction ID
for the
+ // initial charge.
$this->addResponseData( $this->unstageKeys(
$response ) );
+ // FIXME: Silly to store this thing in two
places.
+ // The queue message functions look for it in
transaction_response.
+ // We should make all the gateways store it in
the standard
+ // data fields and get rid of the
transaction_response field.
+
$this->transaction_response->setGatewayTransactionId(
+ $this->getData_Unstaged_Escaped(
'gateway_txn_id' )
+ );
// FIXME: Not a satisfying ending. Parse the
PROFILESTATUS
// response and sort it into complete or
pending.
@@ -444,20 +452,15 @@
$this->checkResponseAck( $response );
$this->addResponseData( $this->unstageKeys(
$response ) );
- // FIXME: Silly.
+ // FIXME: Silly to store this thing in two
places (and say this twice).
+ // See comment in
CreateRecurringPaymentsProfile case
$this->transaction_response->setGatewayTransactionId(
$this->getData_Unstaged_Escaped(
'gateway_txn_id' ) );
$status = $this->findCodeAction(
'DoExpressCheckoutPayment',
'PAYMENTINFO_0_ERRORCODE',
$response['PAYMENTINFO_0_ERRORCODE'] );
- // For recurring payments, we don't want to
finalize or send the queue
- // message just yet
- if (
- $status === FinalStatus::FAILED ||
- !$this->getData_Unstaged_Escaped(
'recurring' )
- ) {
- $this->finalizeInternalStatus( $status
);
- $this->postProcessDonation();
- }
+
+ $this->finalizeInternalStatus( $status );
+ $this->postProcessDonation();
break;
}
@@ -487,7 +490,8 @@
case '10486':
// Donor's first funding method
failed, but they might have another
$this->transaction_response->setRedirect(
-
$this->account_config['RedirectURL'] . $response['TOKEN']
+
$this->account_config['RedirectURL'] .
+
$this->getData_Unstaged_Escaped( 'gateway_session_id' )
);
$fatal = false;
break;
@@ -543,30 +547,23 @@
ResponseCodes::UNKNOWN );
}
- // One-time payment, or initial payment in a subscription.
- $resultData = $this->do_transaction( 'DoExpressCheckoutPayment'
);
- if ( !$resultData->getCommunicationStatus() ) {
- $this->finalizeInternalStatus( FinalStatus::FAILED );
- return PaymentResult::newFailure();
- }
-
- // Silly conditional. What we really want to know is if the
- // DoExpressCheckoutPayment txn was successful.
- if (
- !$resultData->getRedirect() &&
- !$resultData->getErrors() &&
- $this->getData_Unstaged_Escaped( 'recurring' )
- ) {
+ if ( $this->getData_Unstaged_Escaped( 'recurring' ) ) {
// Set up recurring billing agreement.
$this->addRequestData( array(
- // Start in a month; we're making today's
payment as an one-time charge.
- 'date' => time() + 30 * 24 * 3600, // FIXME:
calendar month
+ 'date' => time()
) );
$resultData = $this->do_transaction(
'CreateRecurringPaymentsProfile' );
if ( !$resultData->getCommunicationStatus() ) {
throw new ResponseProcessingException(
'Failed to create a recurring profile',
ResponseCodes::UNKNOWN );
}
+ } else {
+ // One-time payment, or initial payment in a
subscription.
+ $resultData = $this->do_transaction(
'DoExpressCheckoutPayment' );
+ if ( !$resultData->getCommunicationStatus() ) {
+ $this->finalizeInternalStatus(
FinalStatus::FAILED );
+ return PaymentResult::newFailure();
+ }
}
return PaymentResult::fromResults(
$this->getTransactionResponse(),
diff --git a/tests/phpunit/Adapter/PayPal/PayPalApiTest.php
b/tests/phpunit/Adapter/PayPal/PayPalApiTest.php
index 86925a1..6e5445c 100644
--- a/tests/phpunit/Adapter/PayPal/PayPalApiTest.php
+++ b/tests/phpunit/Adapter/PayPal/PayPalApiTest.php
@@ -109,4 +109,67 @@
);
$this->assertEmpty( $logged, 'Logs are a lie, we did not
redirect' );
}
+
+ public function testGoodRecurringSubmit() {
+ $init = array(
+ 'amount' => 1.55,
+ 'currency' => 'USD',
+ 'payment_method' => 'paypal',
+ 'utm_source' => 'CD1234_FR',
+ 'utm_medium' => 'sitenotice',
+ 'country' => 'US',
+ 'recurring' => '1',
+ 'contribution_tracking_id' => strval( mt_rand() ),
+ 'language' => 'fr',
+ );
+ $init['gateway'] = 'paypal_ec';
+ $init['action'] = 'donate';
+
+ $apiResult = $this->doApiRequest( $init );
+ $result = $apiResult[0]['result'];
+ $this->assertTrue( empty( $result['errors'] ) );
+
+ $expectedUrl =
'https://www.sandbox.paypal.com/cgi-bin/webscr?cmd=_express-checkout&token=EC-8US12345X1234567U';
+ $this->assertEquals( $expectedUrl, $result['formaction'],
'PayPal Express API not setting formaction' );
+ $this->assertTrue( $result['status'], 'PayPal Express result
status should be true' );
+
+ $message = DonationQueue::instance()->pop( 'pending' );
+ $this->assertNotEmpty( $message, 'Missing pending message' );
+ DonationInterfaceTestCase::unsetVariableFields( $message );
+ $expected = array(
+ 'country' => 'US',
+ 'fee' => 0,
+ 'gateway' => 'paypal_ec',
+ 'gateway_txn_id' => null,
+ 'language' => 'fr',
+ 'utm_source' => 'CD1234_FR..rpaypal',
+ 'currency' => 'USD',
+ 'email' => '',
+ 'gross' => '1.55',
+ 'recurring' => '1',
+ 'response' => false,
+ 'utm_medium' => 'sitenotice',
+ 'payment_method' => 'paypal',
+ 'payment_submethod' => '',
+ 'gateway_session_id' => 'EC-8US12345X1234567U',
+ 'user_ip' => '127.0.0.1',
+ );
+ // FIXME: want to assert stuff about countribution_tracking_id,
but we
+ // have no way of overriding that for API tests
+ $this->assertArraySubset(
+ $expected,
+ $message,
+ 'PayPal EC setup sending wrong pending message'
+ );
+ $message = DonationQueue::instance()->pop( 'pending' );
+ $this->assertNull( $message, 'Sending extra pending messages' );
+ $logged = DonationInterfaceTestCase::getLogMatches(
+ LogLevel::INFO, '/^Redirecting for transaction: /'
+ );
+ $this->assertEquals( 1, count( $logged ), 'Should have logged
details once' );
+ preg_match( '/Redirecting for transaction: (.*)$/', $logged[0],
$matches );
+ $detailString = $matches[1];
+ $actual = json_decode( $detailString, true );
+ $this->assertArraySubset( $expected, $actual, 'Logged the wrong
stuff!' );
+ }
}
diff --git a/tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
b/tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
index b85a39c..a2a3e63 100644
--- a/tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
+++ b/tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
@@ -45,7 +45,7 @@
$gateway = $this->getFreshGatewayObject( $init );
$gateway->setDummyGatewayResponseCode( 'OK' );
$result = $gateway->doPayment();
- $gateway->logPending(); // GatewayPage calls this for redirects
+ $gateway->logPending(); // GatewayPage or the API calls this
for redirects
$this->assertEquals(
'https://www.sandbox.paypal.com/cgi-bin/webscr?cmd=_express-checkout&token=EC-8US12345X1234567U',
$result->getRedirect(),
@@ -83,6 +83,59 @@
);
}
+ function testPaymentSetupRecurring() {
+ $init = array(
+ 'amount' => 1.55,
+ 'currency' => 'USD',
+ 'payment_method' => 'paypal',
+ 'utm_source' => 'CD1234_FR',
+ 'utm_medium' => 'sitenotice',
+ 'country' => 'US',
+ 'recurring' => '1',
+ 'contribution_tracking_id' => strval( mt_rand() ),
+ 'language' => 'fr',
+ );
+ $gateway = $this->getFreshGatewayObject( $init );
+ $gateway->setDummyGatewayResponseCode( 'OK' );
+ $result = $gateway->doPayment();
+ $gateway->logPending(); // GatewayPage or the API calls this
for redirects
+ $this->assertEquals(
+
'https://www.sandbox.paypal.com/cgi-bin/webscr?cmd=_express-checkout&token=EC-8US12345X1234567U',
+ $result->getRedirect(),
+ 'Wrong redirect for PayPal EC payment setup'
+ );
+ $message = DonationQueue::instance()->pop( 'pending' );
+ $this->assertNotEmpty( $message, 'Missing pending message' );
+ $this->unsetVariableFields( $message );
+ $expected = array(
+ 'country' => 'US',
+ 'fee' => '0',
+ 'gateway' => 'paypal_ec',
+ 'gateway_txn_id' => null,
+ 'language' => 'fr',
+ 'contribution_tracking_id' =>
$init['contribution_tracking_id'],
+ 'order_id' => $init['contribution_tracking_id'] . '.0',
+ 'utm_source' => 'CD1234_FR..rpaypal',
+ 'currency' => 'USD',
+ 'email' => '',
+ 'gross' => '1.55',
+ 'recurring' => '1',
+ 'response' => false,
+ 'utm_medium' => 'sitenotice',
+ 'payment_method' => 'paypal',
+ 'payment_submethod' => '',
+ 'gateway_session_id' => 'EC-8US12345X1234567U',
+ 'user_ip' => '127.0.0.1',
+ 'source_name' => 'DonationInterface',
+ 'source_type' => 'payments',
+ );
+ $this->assertEquals(
+ $expected,
+ $message,
+ 'PayPal EC setup sending wrong pending message'
+ );
+ }
+
/**
* Check that the adapter makes the correct calls for successful
donations
* and sends a good queue message.
diff --git
a/tests/phpunit/includes/Responses/paypal_ec/CreateRecurringPaymentsProfile_10486.testresponse
b/tests/phpunit/includes/Responses/paypal_ec/CreateRecurringPaymentsProfile_10486.testresponse
new file mode 100644
index 0000000..647e408
--- /dev/null
+++
b/tests/phpunit/includes/Responses/paypal_ec/CreateRecurringPaymentsProfile_10486.testresponse
@@ -0,0 +1 @@
+TIMESTAMP=2017%2d04%2d20T16%3a59%3a06Z&CORRELATIONID=537ffff0fefa&ACK=Failure&VERSION=204&BUILD=32574509&L_ERRORCODE0=10486&L_SHORTMESSAGE0=This%20transaction%20couldn%27t%20be%20completed%2e&L_LONGMESSAGE0=This%20transaction%20couldn%27t%20be%20completed%2e%20Please%20redirect%20your%20customer%20to%20PayPal%2e&L_SEVERITYCODE0=Error
diff --git
a/tests/phpunit/includes/Responses/paypal_ec/CreateRecurringPaymentsProfile_Recurring-OK.testresponse
b/tests/phpunit/includes/Responses/paypal_ec/CreateRecurringPaymentsProfile_Recurring-OK.testresponse
index 77d7506..7617c93 100644
---
a/tests/phpunit/includes/Responses/paypal_ec/CreateRecurringPaymentsProfile_Recurring-OK.testresponse
+++
b/tests/phpunit/includes/Responses/paypal_ec/CreateRecurringPaymentsProfile_Recurring-OK.testresponse
@@ -1 +1 @@
-PROFILEID=I%2d88J1M3DLSF0&PROFILESTATUS=ActiveProfile&TIMESTAMP=2017%2d04%2d18T16%3a45%3a29Z&CORRELATIONID=4312c123aa0f2&ACK=Success&VERSION=204&BUILD=25237094
+TRANSACTIONID=5EJ123456T987654S&PROFILEID=I%2d88J1M3DLSF0&PROFILESTATUS=ActiveProfile&TIMESTAMP=2017%2d04%2d18T16%3a45%3a29Z&CORRELATIONID=4312c123aa0f2&ACK=Success&VERSION=204&BUILD=25237094
diff --git
a/tests/phpunit/includes/Responses/paypal_ec/SetExpressCheckout_recurring_OK.testresponse
b/tests/phpunit/includes/Responses/paypal_ec/SetExpressCheckout_recurring_OK.testresponse
new file mode 100644
index 0000000..414758b
--- /dev/null
+++
b/tests/phpunit/includes/Responses/paypal_ec/SetExpressCheckout_recurring_OK.testresponse
@@ -0,0 +1 @@
+TOKEN=EC%2d8US12345X1234567U&TIMESTAMP=2017%2d05%2d18T14%3a53%3a29Z&CORRELATIONID=6d987654a7aed&ACK=Success&VERSION=204&BUILD=33490839
\ No newline at end of file
--
To view, visit https://gerrit.wikimedia.org/r/364903
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifebc76e0e501303381ff8c73000d02e36cdb796c
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <ej...@ejegg.com>
Gerrit-Reviewer: AndyRussG <andrew.green...@gmail.com>
Gerrit-Reviewer: Cdentinger <cdentin...@wikimedia.org>
Gerrit-Reviewer: Eileen <emcnaugh...@wikimedia.org>
Gerrit-Reviewer: Ejegg <ej...@ejegg.com>
Gerrit-Reviewer: Katie Horn <kh...@wikimedia.org>
Gerrit-Reviewer: Mepps <me...@wikimedia.org>
Gerrit-Reviewer: XenoRyet <dkozlow...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
