[MediaWiki-commits] [Gerrit] Deploy-only bugfix: never trust db_update - change (wikimedia...crm)

[Adamw (Code Review)]

Adamw has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/104130


Change subject: Deploy-only bugfix: never trust db_update
......................................................................

Deploy-only bugfix: never trust db_update

Turns out, errors are handled by exception anyway.  The result of
db_update is the num_affected, which is unuseful when doing auditing
because often the targeted row will not change.

Change-Id: I36aca2413f2d50a97b16b825cbdf281652f8ee15
---
M sites/all/modules/queue2civicrm/recurring/recurring.module
1 file changed, 7 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/wikimedia/fundraising/crm 
refs/changes/30/104130/1

diff --git a/sites/all/modules/queue2civicrm/recurring/recurring.module 
b/sites/all/modules/queue2civicrm/recurring/recurring.module
index 2f22409..15f560f 100644
--- a/sites/all/modules/queue2civicrm/recurring/recurring.module
+++ b/sites/all/modules/queue2civicrm/recurring/recurring.module
@@ -396,16 +396,12 @@
   
   $dbs = wmf_civicrm_get_dbs();
   $dbs->push( 'civicrm' );
-  $result = db_update( 'civicrm_contribution_recur' )->fields( array(
+  db_update( 'civicrm_contribution_recur' )->fields( array(
     'cancel_date' => wmf_common_date_unix_to_civicrm( $msg[ 'cancel_date' ] ), 
     'end_date' => wmf_common_date_unix_to_civicrm( $msg[ 'cancel_date' ] ), 
   ) )->condition( 'trxn_id', $msg[ 'subscr_id' ] )->execute();
 
-  if ( !$result ) {
-    throw new WmfException( 'INVALID_RECURRING', 'There was a problem updating 
the subscription for cancelation for subscriber id: ' . print_r( $msg[ 
'subscr_id' ], true ));
-  } else {
-    watchdog( 'recurring', 'Succesfuly cancelled subscription for subscriber 
id %subscr_id', array( '%subscr_id' => print_r( $msg[ 'subscr_id' ], true )), 
WATCHDOG_NOTICE );
-  }
+  watchdog( 'recurring', 'Succesfuly cancelled subscription for subscriber id 
%subscr_id', array( '%subscr_id' => print_r( $msg[ 'subscr_id' ], true )), 
WATCHDOG_NOTICE );
 }
 
 /**
@@ -421,16 +417,12 @@
 
   $dbs = wmf_civicrm_get_dbs();
   $dbs->push( 'civicrm' );
-  $result = db_update( 'civicrm_contribution_recur' )->fields( array(
+  db_update( 'civicrm_contribution_recur' )->fields( array(
     'end_date' => wmf_common_date_unix_to_civicrm( time() ),
   ) )->condition( 'trxn_id', $msg[ 'subscr_id' ] )->execute();
   $dbs->pop();
   
-  if ( !$result ) {
-    throw new WmfException( 'INVALID_RECURRING', 'There was a problem updating 
the subscription for EOT for subscription id: %subscr_id' . print_r( $msg[ 
'subscr_id' ], true ));
-  } else {
-    watchdog( 'recurring', 'Succesfuly ended subscription for subscriber id: 
%subscr_id ', array( '%subscr_id' => print_r( $msg[ 'subscr_id' ], true )), 
WATCHDOG_NOTICE );
-  }
+  watchdog( 'recurring', 'Succesfuly ended subscription for subscriber id: 
%subscr_id ', array( '%subscr_id' => print_r( $msg[ 'subscr_id' ], true )), 
WATCHDOG_NOTICE );
 }
 
 /**
@@ -449,7 +441,7 @@
 
   $dbs = wmf_civicrm_get_dbs();
   $dbs->push( 'civicrm' );
-  $result = db_update( 'civicrm_contribution_recur' )->fields( array(
+  db_update( 'civicrm_contribution_recur' )->fields( array(
     'amount' => $msg[ 'original_gross' ],
     'frequency_unit' => $msg[ 'frequency_unit' ],
     'frequency_interval' => $msg[ 'frequency_interval' ],
@@ -459,10 +451,6 @@
   ) )->condition( 'trxn_id', $msg[ 'subscr_id' ] )->execute();
   $dbs->pop();
 
-  if ( !$result ) {
-    throw new WmfException( 'INVALID_RECURRING', 'There was a problem updating 
the subscription record for subscription id ' . print_r( $msg['subscr_id'], 
true ) );
-  }
-   
   // update the contact
   $contact = wmf_civicrm_message_contact_update( $msg, 
$recur_record->contact_id );
   
@@ -487,16 +475,12 @@
   
   $dbs = wmf_civicrm_get_dbs();
   $dbs->push( 'civicrm' );
-  $result = db_update( 'civicrm_contribution_recur' )->fields( array(
+  db_update( 'civicrm_contribution_recur' )->fields( array(
     'failure_count' => $msg[ 'failure_count' ],
     'failure_retry_date' => wmf_common_date_unix_to_civicrm( $msg[ 
'failure_retry_date' ] ),
   ) )->condition( 'trxn_id', $msg[ 'subscr_id' ] )->execute();
 
-  if ( !$result ) {
-    throw new WmfException( 'INVALID_RECURRING', 'There was a problem updating 
the subscription for failed payment for subscriber id: ' . print_r( 
$msg['subscr_id'], true ) );
-  } else {
-    watchdog( 'recurring', 'Succesfuly cacneled subscription for failed 
payment for subscriber id: %subscr_id ', array( '%subscr_id' => print_r( $msg[ 
'subscr_id' ], true )), WATCHDOG_NOTICE );
-  }
+  watchdog( 'recurring', 'Succesfuly cacneled subscription for failed payment 
for subscriber id: %subscr_id ', array( '%subscr_id' => print_r( $msg[ 
'subscr_id' ], true )), WATCHDOG_NOTICE );
 }
 
 /**

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I36aca2413f2d50a97b16b825cbdf281652f8ee15
Gerrit-PatchSet: 1
Gerrit-Project: wikimedia/fundraising/crm
Gerrit-Branch: master
Gerrit-Owner: Adamw <awi...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits