[freeside-commits] branch master updated. 99d2d7c12e04c5ed754122f26f403801dfea2839

[Mark Wells]

The branch, master has been updated
       via  99d2d7c12e04c5ed754122f26f403801dfea2839 (commit)
      from  06583ca8e9590de4bc44e849791166d1b8be90ce (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 99d2d7c12e04c5ed754122f26f403801dfea2839
Author: Mark Wells <mark at freeside.biz>
Date:   Fri Dec 9 15:23:35 2016 -0800

    stop packages from changing pkgpart on cancellation in certain cases, #73607

diff --git a/FS/FS/cust_main/Billing.pm b/FS/FS/cust_main/Billing.pm
index 4821ce5..6932647 100644
--- a/FS/FS/cust_main/Billing.pm
+++ b/FS/FS/cust_main/Billing.pm
@@ -544,14 +544,19 @@ sub bill {
 
     foreach my $part_pkg ( @part_pkg ) {
 
-      $cust_pkg->set($_, $hash{$_}) foreach qw ( setup last_bill bill );
+      my $this_cust_pkg = $cust_pkg;
+      # for add-on packages, copy the object to avoid leaking changes back to
+      # the caller if pkg_list is in use; see RT#73607
+      if ( $part_pkg->get('pkgpart') != $real_pkgpart ) {
+        $this_cust_pkg = FS::cust_pkg->new({ %hash });
+      }
 
       my $pass = '';
-      if ( $cust_pkg->separate_bill ) {
+      if ( $this_cust_pkg->separate_bill ) {
         # if no_auto is also set, that's fine. we just need to not have
         # invoices that are both auto and no_auto, and since the package
         # gets an invoice all to itself, it will only be one or the other.
-        $pass = $cust_pkg->pkgnum;
+        $pass = $this_cust_pkg->pkgnum;
         if (!exists $cust_bill_pkg{$pass}) { # it may not exist yet
           push @passes, $pass;
           $total_setup{$pass} = do { my $z = 0; \$z };
@@ -565,17 +570,17 @@ sub bill {
                                 );
           $cust_bill_pkg{$pass} = [];
         }
-      } elsif ( ($cust_pkg->no_auto || $part_pkg->no_auto) ) {
+      } elsif ( ($this_cust_pkg->no_auto || $part_pkg->no_auto) ) {
         $pass = 'no_auto';
       }
 
-      my $next_bill = $cust_pkg->getfield('bill') || 0;
+      my $next_bill = $this_cust_pkg->getfield('bill') || 0;
       my $error;
       # let this run once if this is the last bill upon cancellation
       while ( $next_bill <= $cmp_time or $options{cancel} ) {
         $error =
           $self->_make_lines( 'part_pkg'            => $part_pkg,
-                              'cust_pkg'            => $cust_pkg,
+                              'cust_pkg'            => $this_cust_pkg,
                               'precommit_hooks'     => \@precommit_hooks,
                               'line_items'          => $cust_bill_pkg{$pass},
                               'setup'               => $total_setup{$pass},
@@ -590,12 +595,12 @@ sub bill {
         last if $error;
 
         # or if we're not incrementing the bill date.
-        last if ($cust_pkg->getfield('bill') || 0) == $next_bill;
+        last if ($this_cust_pkg->getfield('bill') || 0) == $next_bill;
 
         # or if we're letting it run only once
         last if $options{cancel};
 
-        $next_bill = $cust_pkg->getfield('bill') || 0;
+        $next_bill = $this_cust_pkg->getfield('bill') || 0;
 
         #stop if -o was passed to freeside-daily
         last if $options{'one_recur'};
diff --git a/FS/FS/cust_pkg.pm b/FS/FS/cust_pkg.pm
index 4e9ede3..0bdef02 100644
--- a/FS/FS/cust_pkg.pm
+++ b/FS/FS/cust_pkg.pm
@@ -5575,6 +5575,32 @@ sub _upgrade_data {  # class method
     my $error = $part_pkg_link->remove_linked;
     die $error if $error;
   }
+
+  # RT#73607: canceling a package with billing addons sometimes changes its
+  # pkgpart.
+  # Find records where the last replace_new record for the package before it
+  # was canceled has a different pkgpart from the package itself.
+  my @cust_pkg = qsearch({
+    'table' => 'cust_pkg',
+    'select' => 'cust_pkg.*, h_cust_pkg.pkgpart AS h_pkgpart',
+    'addl_from' => ' JOIN (
+  SELECT pkgnum, MAX(historynum) AS historynum FROM h_cust_pkg
+    WHERE cancel IS NULL
+      AND history_action = \'replace_new\'
+    GROUP BY pkgnum
+  ) AS last_history USING (pkgnum)
+  JOIN h_cust_pkg USING (historynum)',
+    'extra_sql' => ' WHERE cust_pkg.cancel is not null
+                     AND cust_pkg.pkgpart != h_cust_pkg.pkgpart'
+  });
+  foreach my $cust_pkg ( @cust_pkg ) {
+    my $pkgnum = $cust_pkg->pkgnum;
+    warn "fixing pkgpart on canceled pkg#$pkgnum\n";
+    $cust_pkg->set('pkgpart', $cust_pkg->h_pkgpart);
+    my $error = $cust_pkg->replace;
+    die $error if $error;
+  }
+
 }
 
 =back

-----------------------------------------------------------------------

Summary of changes:
 FS/FS/cust_main/Billing.pm |   21 +++++++++++++--------
 FS/FS/cust_pkg.pm          |   26 ++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 8 deletions(-)