fix bad transactional decisions that made it possible to abort and rollback a gateway payment, RT#8995

2 files changed, 14 insertions, 22 deletions

diff --git a/FS/FS/cust_event.pm b/FS/FS/cust_event.pm
index 52b5911..1407f43 100644
--- a/FS/FS/cust_event.pm
+++ b/FS/FS/cust_event.pm
@@ -222,9 +222,6 @@ sub do_event {
        " (". $part_event->action. ") $for\n"
     if $DEBUG;
 
-  my $oldAutoCommit = $FS::UID::AutoCommit;
-  local $FS::UID::AutoCommit = 0;
-
   my $error;
   {
     local $SIG{__DIE__}; # don't want Mason __DIE__ handler active
diff --git a/FS/FS/cust_main.pm b/FS/FS/cust_main.pm
index 9c16294..e98f700 100644
--- a/FS/FS/cust_main.pm
+++ b/FS/FS/cust_main.pm
@@ -3696,19 +3696,17 @@ sub collect {
     }
   }
 
-  my $error = $self->do_cust_event(
+  $dbh->commit or die $dbh->errstr if $oldAutoCommit;
+
+  #never want to roll back an event just because it returned an error
+  local $FS::UID::AutoCommit = 1; #$oldAutoCommit;
+
+  $self->do_cust_event(
     'debug'      => ( $options{'debug'} || 0 ),
     'time'       => $invoice_time,
     'check_freq' => $options{'check_freq'},
     'stage'      => 'collect',
   );
-  if ( $error ) {
-    $dbh->rollback if $oldAutoCommit;
-    return $error;
-  }
-
-  $dbh->commit or die $dbh->errstr if $oldAutoCommit;
-  '';
 
 }
 
@@ -3803,6 +3801,11 @@ sub do_cust_event {
     return $due_cust_event;
   }
 
+  $dbh->commit or die $dbh->errstr if $oldAutoCommit;
+  #never want to roll back an event just because it or a different one
+  # returned an error
+  local $FS::UID::AutoCommit = 1; #$oldAutoCommit;
+
   foreach my $cust_event ( @$due_cust_event ) {
 
     #XXX lock event
@@ -3811,11 +3814,7 @@ sub do_cust_event {
     unless ( $cust_event->test_conditions( 'time' => $time ) ) {
       #don't leave stray "new/locked" records around
       my $error = $cust_event->delete;
-      if ( $error ) {
-        #gah, even with transactions
-        $dbh->commit if $oldAutoCommit; #well.
-        return $error;
-      }
+      return $error if $error;
       next;
     }
 
@@ -3824,20 +3823,16 @@ sub do_cust_event {
       warn "  running cust_event ". $cust_event->eventnum. "\n"
         if $DEBUG > 1;
 
-      
       #if ( my $error = $cust_event->do_event(%options) ) { #XXX %options?
       if ( my $error = $cust_event->do_event() ) {
         #XXX wtf is this?  figure out a proper dealio with return value
         #from do_event
-	  # gah, even with transactions.
-	  $dbh->commit if $oldAutoCommit; #well.
-	  return $error;
-	}
+        return $error;
+      }
     }
 
   }
 
-  $dbh->commit or die $dbh->errstr if $oldAutoCommit;
   '';
 
 }