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