jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/328855 )

Change subject: Check contribution_id before rectifying orphans
......................................................................


Check contribution_id before rectifying orphans

If there's already a contribution_id in the contribution_tracking
table, that means the donor somehow made two successful donations
with the same contribution tracking ID. In almost all cases this
is inadvertant, so cancel the second one.

Needs tests.

Bug: T153992
Change-Id: I2d0c635148844978f0b856f58b95076d939eb174
---
M globalcollect_gateway/GlobalCollectOrphanRectifier.php
M globalcollect_gateway/orphan.adapter.php
M tests/phpunit/Adapter/GlobalCollect/GlobalCollectOrphanAdapterTest.php
3 files changed, 31 insertions(+), 32 deletions(-)

Approvals:
  Cdentinger: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/globalcollect_gateway/GlobalCollectOrphanRectifier.php 
b/globalcollect_gateway/GlobalCollectOrphanRectifier.php
index 4de9e49..ac404b1 100644
--- a/globalcollect_gateway/GlobalCollectOrphanRectifier.php
+++ b/globalcollect_gateway/GlobalCollectOrphanRectifier.php
@@ -36,7 +36,7 @@
        protected $time_buffer;
 
        /**
-        * @var GatewayType Payments adapter to do the processing.
+        * @var GlobalCollectOrphanAdapter Payments adapter to do the 
processing.
         */
        protected $adapter;
 
@@ -156,17 +156,25 @@
         * fully rectified or not.
         *
         * @param array $normalized Orphaned message
-        * @param boolean $query_contribution_tracking A flag specifying if we
-        * should query the contribution_tracking table or not.  Defaults to 
true.
         *
         * @return boolean True if the orphan has been rectified, false if not.
         */
-       protected function rectifyOrphan( $normalized, 
$query_contribution_tracking = true ){
+       protected function rectifyOrphan( $normalized ){
                $this->logger->info( "Rectifying orphan: 
{$normalized['order_id']}" );
                $is_rectified = false;
 
-               $this->adapter->loadDataAndReInit( $normalized, 
$query_contribution_tracking );
-               $results = $this->adapter->do_transaction( 'Confirm_CreditCard' 
);
+               $this->adapter->loadDataAndReInit( $normalized );
+               $civiId = $this->adapter->getData_Unstaged_Escaped( 
'contribution_id' );
+               if ( $civiId ) {
+                       $this->logger->error(
+                               $normalized['contribution_tracking_id'] .
+                               ": Contribution tracking already has 
contribution_id $civiId.  " .
+                               'Stop confusing donors!'
+                       );
+                       $results = $this->adapter->do_transaction( 
'CANCEL_PAYMENT' );
+               } else {
+                       $results = $this->adapter->do_transaction( 
'Confirm_CreditCard' );
+               }
 
                // FIXME: error message is squishy and inconsistent with the 
error_map
                // used elsewhere.
diff --git a/globalcollect_gateway/orphan.adapter.php 
b/globalcollect_gateway/orphan.adapter.php
index 296b0d3..eb85ba8 100644
--- a/globalcollect_gateway/orphan.adapter.php
+++ b/globalcollect_gateway/orphan.adapter.php
@@ -2,7 +2,7 @@
 
 class GlobalCollectOrphanAdapter extends GlobalCollectAdapter {
        //Data we know to be good, that we always want to re-assert after a 
load or an addData.
-       //so far: order_id and the utm data we pull from contribution tracking.
+       //so far: order_id and the data we pull from contribution tracking.
        protected $hard_data = array ( );
 
        public static function getLogIdentifier() {
@@ -51,33 +51,21 @@
        }
 
        // FIXME: This needs some serious code reuse trickery.
-       public function loadDataAndReInit( $data, $useDB = true ) {
+       public function loadDataAndReInit( $data ) {
                //re-init all these arrays, because this is a batch thing.
                $this->session_killAllEverything(); // just to be sure
                $this->transaction_response = new PaymentTransactionResponse();
-               $this->hard_data = array( );
+               $this->hard_data = array(
+                       'order_id' => $data['order_id']
+               );
                $this->unstaged_data = array( );
                $this->staged_data = array( );
-
-               $this->hard_data['order_id'] = $data['order_id'];
 
                $this->dataObj = new DonationData( $this, $data );
 
                $this->unstaged_data = $this->dataObj->getDataEscaped();
 
-               if ( $useDB ){
-                       $this->hard_data = array_merge( $this->hard_data, 
$this->getUTMInfoFromDB() );
-               } else {
-                       $utm_keys = array(
-                               'utm_source',
-                               'utm_campaign',
-                               'utm_medium',
-                               'date'
-                       );
-                       foreach($utm_keys as $key){
-                               $this->hard_data[$key] = $data[$key];
-                       }
-               }
+               $this->hard_data = array_merge( $this->hard_data, 
$this->getContributionTracking() );
                $this->reAddHardData();
 
                $this->staged_data = $this->unstaged_data;
@@ -115,7 +103,7 @@
                }
        }
 
-       public function getUTMInfoFromDB() {
+       public function getContributionTracking() {
                if ( $this->getData_Unstaged_Escaped( 'utm_source' ) ) {
                        // We already have the info.
                        return array();
@@ -140,6 +128,7 @@
                        $res = $db->select(
                                'contribution_tracking',
                                array(
+                                       'contribution_id',
                                        'utm_source',
                                        'utm_campaign',
                                        'utm_medium',
@@ -147,7 +136,9 @@
                                ),
                                array( 'id' => $ctid )
                        );
+                       // Fixme: if we get more than one row back, MySQL is 
broken
                        foreach ( $res as $thing ) {
+                               $data['contribution_id'] = 
$thing->contribution_id;
                                $data['utm_source'] = $thing->utm_source;
                                $data['utm_campaign'] = $thing->utm_campaign;
                                $data['utm_medium'] = $thing->utm_medium;
@@ -162,7 +153,7 @@
                }
 
                //if we got here, we can't find anything else...
-               $this->logger->error( "$ctid: FAILED to find UTM Source value. 
Using default." );
+               $this->logger->error( "$ctid: FAILED to find contribution 
tracking data. Using default." );
                return $data;
        }
 
diff --git 
a/tests/phpunit/Adapter/GlobalCollect/GlobalCollectOrphanAdapterTest.php 
b/tests/phpunit/Adapter/GlobalCollect/GlobalCollectOrphanAdapterTest.php
index 3de7a8d..6682a87 100644
--- a/tests/phpunit/Adapter/GlobalCollect/GlobalCollectOrphanAdapterTest.php
+++ b/tests/phpunit/Adapter/GlobalCollect/GlobalCollectOrphanAdapterTest.php
@@ -83,11 +83,11 @@
                $data['order_id'] = '55555';
 
                //now, add data and check that we didn't kill the oid. Still 
generating.
-               $gateway->loadDataAndReInit( $data, $useDB = false );
+               $gateway->loadDataAndReInit( $data );
                $this->assertEquals( $gateway->getData_Unstaged_Escaped( 
'order_id' ), '55555', 'loadDataAndReInit failed to stick OrderID' );
 
                $data['order_id'] = '444444';
-               $gateway->loadDataAndReInit( $data, $useDB = false );
+               $gateway->loadDataAndReInit( $data );
                $this->assertEquals( $gateway->getData_Unstaged_Escaped( 
'order_id' ), '444444', 'loadDataAndReInit failed to stick OrderID' );
 
                $this->verifyNoLogErrors();
@@ -104,11 +104,11 @@
                $data['order_id'] = '66666';
 
                //now, add data and check that we didn't kill the oid. Still 
not generating
-               $gateway->loadDataAndReInit( $data, $useDB = false );
+               $gateway->loadDataAndReInit( $data );
                $this->assertEquals( $gateway->getData_Unstaged_Escaped( 
'order_id' ), '66666', 'loadDataAndReInit failed to stick OrderID' );
 
                $data['order_id'] = '777777';
-               $gateway->loadDataAndReInit( $data, $useDB = false );
+               $gateway->loadDataAndReInit( $data );
                $this->assertEquals( $gateway->getData_Unstaged_Escaped( 
'order_id' ), '777777', 'loadDataAndReInit failed to stick OrderID on second 
batch item' );
 
                $this->verifyNoLogErrors();
@@ -156,7 +156,7 @@
                $init['order_id'] = '55555';
                $init['email'] = 'innoc...@clean.com';
                $init['contribution_tracking_id'] = mt_rand();
-               $gateway->loadDataAndReInit( $init, $useDB = false );
+               $gateway->loadDataAndReInit( $init );
 
                $gateway->setDummyGatewayResponseCode( $code );
                $result = $gateway->do_transaction( 'Confirm_CreditCard' );
@@ -190,7 +190,7 @@
                $init['contribution_tracking_id'] = mt_rand();
                $init['payment_method'] = 'cc';
 
-               $gateway->loadDataAndReInit( $init, $useDB = false );
+               $gateway->loadDataAndReInit( $init );
                $gateway->setDummyGatewayResponseCode( '600_badCvv' );
 
                $gateway->do_transaction( 'Confirm_CreditCard' );

-- 
To view, visit https://gerrit.wikimedia.org/r/328855
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I2d0c635148844978f0b856f58b95076d939eb174
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: AndyRussG <andrew.green...@gmail.com>
Gerrit-Reviewer: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: Cdentinger <cdentin...@wikimedia.org>
Gerrit-Reviewer: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: Ssmith <ssm...@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

Reply via email to