jenkins-bot has submitted this change and it was merged. Change subject: Update Paypal return URL handling ......................................................................
Update Paypal return URL handling We were never sending users back through the resultswitcher, and it didn't have any useful logic in it. We haven't been sending any return or cancel_return parameters, instead relying on values set in the merchant console. This change uses the standard getThankYouPage instead of a custom PaypalGatewayReturnURL global, and adds a distinct cancel URL for users who decide to give another amount or by another method. Using the standard thank you page logic should ensure that we get the country parameter on the thank you page. Bug: T126814 Change-Id: I6807c8341f3b45518420f0fecc2c5e6cde56d4df --- M DonationInterface.php M paypal_gateway/paypal.adapter.php D paypal_gateway/paypal_resultswitcher.body.php D tests/Adapter/PayPal/PayPalResultSwitcherTest.php M tests/Adapter/PayPal/PayPalTest.php 5 files changed, 28 insertions(+), 142 deletions(-) Approvals: Awight: Looks good to me, approved jenkins-bot: Verified diff --git a/DonationInterface.php b/DonationInterface.php index f323b5a..70d76b1 100644 --- a/DonationInterface.php +++ b/DonationInterface.php @@ -94,7 +94,6 @@ // Paypal $wgAutoloadClasses['PaypalGateway'] = __DIR__ . '/paypal_gateway/paypal_gateway.body.php'; -$wgAutoloadClasses['PaypalGatewayResult'] = __DIR__ . '/paypal_gateway/paypal_resultswitcher.body.php'; $wgAutoloadClasses['PaypalAdapter'] = __DIR__ . '/paypal_gateway/paypal.adapter.php'; // Worldpay @@ -347,7 +346,7 @@ $wgPaypalGatewayURL = 'https://www.paypal.com/cgi-bin/webscr'; $wgPaypalGatewayTestingURL = 'https://www.sandbox.paypal.com/cgi-bin/webscr'; -$wgPaypalGatewayReturnURL = ''; //'http://127.0.0.1/index.php/Special:PaypalGatewayResult'; +$wgPaypalGatewayCancelPage = ''; //https://wikimediafoundation.org/wiki/Ways_to_Give $wgPaypalGatewayRecurringLength = '0'; // 0 should mean forever $wgPaypalGatewayHtmlFormDir = __DIR__ . '/paypal_gateway/forms/html'; @@ -836,7 +835,6 @@ $wgDonationInterfaceGatewayAdapters[] = 'AstropayAdapter'; $wgSpecialPages['PaypalGateway'] = 'PaypalGateway'; -$wgSpecialPages['PaypalGatewayResult'] = 'PaypalGatewayResult'; $wgDonationInterfaceGatewayAdapters[] = 'PaypalAdapter'; $wgSpecialPages['WorldpayGateway'] = 'WorldpayGateway'; diff --git a/paypal_gateway/paypal.adapter.php b/paypal_gateway/paypal.adapter.php index 5ba6697..5ea4015 100644 --- a/paypal_gateway/paypal.adapter.php +++ b/paypal_gateway/paypal.adapter.php @@ -113,12 +113,12 @@ ), 'values' => array( 'business' => $this->account_config[ 'AccountEmail' ], - 'cancel_return' => $this->getGlobal( 'ReturnURL' ), + 'cancel_return' => $this->getCancelPage(), 'cmd' => '_donations', 'item_number' => 'DONATE', 'item_name' => WmfFramework::formatMessage( 'donate_interface-donation-description' ), 'no_note' => 0, - 'return' => $this->getGlobal( 'ReturnURL' ), + 'return' => $this->getThankYouPage(), ), ); $this->transactions[ 'DonateXclick' ] = array( @@ -140,9 +140,9 @@ 'values' => array( 'item_number' => 'DONATE', 'item_name' => WmfFramework::formatMessage( 'donate_interface-donation-description' ), - 'cancel_return' => $this->getGlobal( 'ReturnURL' ), + 'cancel_return' => $this->getCancelPage(), 'no_note' => '1', - 'return' => $this->getGlobal( 'ReturnURL' ), + 'return' => $this->getThankYouPage(), 'business' => $this->account_config[ 'AccountEmail' ], 'cmd' => '_xclick', 'no_shipping' => '1' @@ -169,12 +169,12 @@ ), 'values' => array( 'business' => $this->account_config[ 'AccountEmail' ], - 'cancel_return' => $this->getGlobal( 'ReturnURL' ), + 'cancel_return' => $this->getCancelPage(), 'cmd' => '_xclick-subscriptions', 'item_number' => 'DONATE', 'item_name' => WmfFramework::formatMessage( 'donate_interface-donation-description' ), 'no_note' => 0, - 'return' => $this->getGlobal( 'ReturnURL' ), + 'return' => $this->getThankYouPage(), // recurring fields 't3' => 'M', 'p3' => '1', @@ -316,4 +316,12 @@ } } } + + public function getCancelPage() { + $cancelPage = $this->getGlobal( 'CancelPage' ); + if ( empty( $cancelPage ) ) { + return ''; + } + return $this->appendLanguageAndMakeURL( $cancelPage ); + } } diff --git a/paypal_gateway/paypal_resultswitcher.body.php b/paypal_gateway/paypal_resultswitcher.body.php deleted file mode 100644 index fcadf0c..0000000 --- a/paypal_gateway/paypal_resultswitcher.body.php +++ /dev/null @@ -1,75 +0,0 @@ -<?php - -class PaypalGatewayResult extends GatewayPage { - - /** - * Defines the action to take on a Paypal transaction. - * - * Possible values include 'process', 'challenge', - * 'review', 'reject'. These values can be set during - * data processing validation, for instance. - * - * Hooks are exposed to handle the different actions. - * - * Defaults to 'process'. - * @var string - */ - public $action = 'process'; - - /** - * An array of form errors - * @var array - */ - public $errors = array(); - - public function __construct() { - $this->adapter = new PaypalAdapter(); - parent::__construct(); - } - - /** - * Show the special page - */ - protected function handleRequest() { - // no longer letting people in without these things. - // If this is preventing you from doing something, - // you almost certainly want to be somewhere else. - $forbidden = false; - if ( !$this->adapter->session_hasDonorData() ) { - $forbidden = true; - $f_message = 'No active donation in the session'; - } - - if ( $forbidden ){ - wfHttpError( 403, 'Forbidden', wfMessage( 'donate_interface-error-http-403' )->text() ); - } - $oid = $this->adapter->getData_Unstaged_Escaped( 'order_id' ); - - $this->setHeaders(); - - if ( $this->adapter->checkTokens() ) { - - /* - // One day, we should have pass/fail detection. - // I don't think PP returns enough information at the moment. - if ( NULL === $this->adapter->processResponse( $ ) ) { - switch ( $this->adapter->getFinalStatus() ) { - case 'complete': - case 'pending': - $this->getOutput()->redirect( $this->adapter->getThankYouPage() ); - return; - } - } - $this->getOutput()->redirect( $this->adapter->getFailPage() ); - */ - $this->logger->info( "Displaying thank you page" ); - $this->getOutput()->redirect( $this->adapter->getThankYouPage() ); - } else { - $this->logger->info( "Resultswitcher: Token Check Failed. Order ID: $oid" ); - $this->displayFailPage(); - } - } - -} - -// end class diff --git a/tests/Adapter/PayPal/PayPalResultSwitcherTest.php b/tests/Adapter/PayPal/PayPalResultSwitcherTest.php deleted file mode 100644 index 7021066..0000000 --- a/tests/Adapter/PayPal/PayPalResultSwitcherTest.php +++ /dev/null @@ -1,46 +0,0 @@ -<?php -/** - * Wikimedia Foundation - * - * LICENSE - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -/** - * @group Fundraising - * @group DonationInterface - * @group PayPal - */ -class PayPalResultSwitcherTest extends DonationInterfaceTestCase { - public function setUp() { - parent::setUp(); - - $this->setMwGlobals( array( - 'wgPaypalGatewayEnabled' => true, - ) ); - } - - function testSuccessfulRedirect() { - $init = $this->getDonorTestData( 'FR' ); - $init['OTT'] = 'SALT123456789'; - $session = array( 'Donor' => $init ); - - $assertNodes = array( - 'headers' => array( - 'Location' => 'https://wikimediafoundation.org/wiki/Thank_You/fr?country=FR', - ), - ); - - $this->verifyFormOutput( 'PaypalGatewayResult', $init, $assertNodes, false, $session ); - } - -} diff --git a/tests/Adapter/PayPal/PayPalTest.php b/tests/Adapter/PayPal/PayPalTest.php index e944004..a6dd8b1 100644 --- a/tests/Adapter/PayPal/PayPalTest.php +++ b/tests/Adapter/PayPal/PayPalTest.php @@ -39,6 +39,7 @@ parent::setUp(); $this->setMwGlobals( array( + 'wgPaypalGatewayCancelPage' => 'https://example.com/tryAgain.php', 'wgPaypalGatewayEnabled' => true, 'wgDonationInterfaceAllowedHtmlForms' => array( 'paypal' => array( @@ -83,8 +84,8 @@ 'no_note' => '0', 'custom' => $gateway->getData_Unstaged_Escaped( 'contribution_tracking_id' ), 'lc' => $init['country'], //this works because it's a US donor... - 'cancel_return' => $gateway->getGlobal( 'ReturnURL' ), - 'return' => $gateway->getGlobal( 'ReturnURL' ), + 'cancel_return' => $gateway->getCancelPage(), + 'return' => $gateway->getThankYouPage(), ); $this->assertEquals( $expected, $res, 'Paypal "Donate" transaction not constructing the expected redirect URL' ); @@ -117,8 +118,8 @@ 'p3' => '1', //hard-coded in transaction definition 'src' => '1', //hard-coded in transaction definition 'srt' => $gateway->getGlobal( 'RecurringLength' ), - 'cancel_return' => $gateway->getGlobal( 'ReturnURL' ), - 'return' => $gateway->getGlobal( 'ReturnURL' ), + 'cancel_return' => $gateway->getCancelPage(), + 'return' => $gateway->getThankYouPage(), ); $this->assertEquals( $expected, $res, 'Paypal "DonateRecurring" transaction not constructing the expected redirect URL' ); @@ -150,8 +151,8 @@ 'no_note' => '1', //hard-coded in transaction definition 'custom' => $gateway->getData_Unstaged_Escaped( 'contribution_tracking_id' ), // 'lc' => $init['country'], //Apparently, this was removed from our implementation, because 'CN' is weird. - 'cancel_return' => $gateway->getGlobal( 'ReturnURL' ), - 'return' => $gateway->getGlobal( 'ReturnURL' ), + 'cancel_return' => $gateway->getCancelPage(), + 'return' => $gateway->getThankYouPage(), 'no_shipping' => '1', //hard-coded in transaction definition ); @@ -221,8 +222,8 @@ 'no_note' => '0', 'custom' => $gateway->getData_Unstaged_Escaped( 'contribution_tracking_id' ), 'lc' => $init['country'], //this works because it's a US donor... - 'cancel_return' => $gateway->getGlobal( 'ReturnURL' ), - 'return' => $gateway->getGlobal( 'ReturnURL' ), + 'cancel_return' => $gateway->getCancelPage(), + 'return' => $gateway->getThankYouPage(), ); $this->assertEquals( $expected, $res, 'Paypal "Donate" transaction not constructing the expected redirect URL' ); @@ -255,8 +256,8 @@ 'no_note' => '0', 'custom' => $gateway->getData_Unstaged_Escaped( 'contribution_tracking_id' ), 'lc' => 'CA', - 'cancel_return' => $gateway->getGlobal( 'ReturnURL' ), - 'return' => $gateway->getGlobal( 'ReturnURL' ), + 'cancel_return' => $gateway->getCancelPage(), + 'return' => $gateway->getThankYouPage(), ); $this->assertEquals( $expected, $res, 'Paypal "Donate" transaction not constructing the expected redirect URL' ); @@ -285,8 +286,8 @@ 'no_note' => '0', 'custom' => $gateway->getData_Unstaged_Escaped( 'contribution_tracking_id' ), 'lc' => 'IT', - 'cancel_return' => $gateway->getGlobal( 'ReturnURL' ), - 'return' => $gateway->getGlobal( 'ReturnURL' ), + 'cancel_return' => $gateway->getCancelPage(), + 'return' => $gateway->getThankYouPage(), ); $this->assertEquals( $expected, $res, 'Paypal "Donate" transaction not constructing the expected redirect URL' ); -- To view, visit https://gerrit.wikimedia.org/r/273966 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I6807c8341f3b45518420f0fecc2c5e6cde56d4df Gerrit-PatchSet: 3 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: Ssmith <[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
