use credit_lineitems logic for unused-time credits, #42729
authorMark Wells <mark@freeside.biz>
Wed, 21 Sep 2016 17:20:18 +0000 (10:20 -0700)
committerMark Wells <mark@freeside.biz>
Wed, 21 Sep 2016 17:20:18 +0000 (10:20 -0700)
FS/FS/cust_credit.pm
FS/FS/cust_pkg.pm
FS/t/suite/09-sales_tax_credit_change.t [new file with mode: 0755]

index e4b1fc0..4860315 100644 (file)
@@ -783,7 +783,11 @@ sub calculate_tax_adjustment {
   );
 }
 
-=item credit_lineitems
+=item credit_lineitems OPTIONS
+
+Creates a credit to a group of line items, with a specified amount applied
+to each. This will also calculate the tax adjustments for those amounts and
+credit the appropriate tax line items.
 
 Example:
 
@@ -802,6 +806,16 @@ Example:
 
   );
 
+C<billpkgnums>, C<setuprecurs>, C<amounts> are required and are parallel
+arrays. Each one indicates an amount of credit to be applied to either the
+setup or recur portion of a (non-tax) line item.
+
+C<custnum>, C<_date>, C<reasonnum>, and C<addlinfo> will be set on the
+credit before it's inserted.
+
+C<amount> is the total amount. If unspecified, the credit will be the sum
+of the per-line-item amounts and their tax adjustments.
+
 =cut
 
 #maybe i should just be an insert with extra args instead of a class method
@@ -841,10 +855,18 @@ sub credit_lineitems {
 
   my $error = '';
 
+  # first, determine the tax adjustments
+  my %tax_adjust = $class->calculate_tax_adjustment(%arg);
+  # and determine the amount automatically if it wasn't specified
+  if ( !exists( $arg{amount} ) ) {
+    $arg{amount} = sprintf('%.2f', $tax_adjust{subtotal} + $tax_adjust{taxtotal});
+  }
+
+  # create the credit
   my $cust_credit = new FS::cust_credit ( {
     map { $_ => $arg{$_} }
       #fields('cust_credit')
-      qw( custnum _date amount reasonnum addlinfo ), #pkgnum eventnum
+      qw( custnum _date amount reason reasonnum addlinfo ), #pkgnum eventnum
   } );
   $error = $cust_credit->insert;
   if ( $error ) {
@@ -864,9 +886,6 @@ sub credit_lineitems {
   my %cust_credit_bill_pkg = ();
   my %unapplied_payments = (); #invoice numbers, and then billpaynums
 
-  # determine the tax adjustments
-  my %tax_adjust = $class->calculate_tax_adjustment(%arg);
-
   foreach my $billpkgnum ( @{$arg{billpkgnums}} ) {
     my $setuprecur = shift @{$arg{setuprecurs}};
     my $amount = shift @{$arg{amounts}};
index c8c5484..e9d80da 100644 (file)
@@ -8,7 +8,7 @@ use base qw( FS::cust_pkg::Search FS::cust_pkg::API
 use strict;
 use Carp qw(cluck);
 use Scalar::Util qw( blessed );
-use List::Util qw(min max);
+use List::Util qw(min max sum);
 use Tie::IxHash;
 use Time::Local qw( timelocal timelocal_nocheck );
 use MIME::Entity;
@@ -1774,50 +1774,93 @@ sub credit_remaining {
   my $conf = FS::Conf->new;
   my $reason_type = $conf->config($mode.'_credit_type');
 
-  my $last_bill = $self->getfield('last_bill') || 0;
-  my $next_bill = $self->getfield('bill') || 0;
-  if ( $last_bill > 0         # the package has been billed
-      and $next_bill > 0      # the package has a next bill date
-      and $next_bill >= $time # which is in the future
-  ) {
-    my @cust_credit_source_bill_pkg = ();
-    my $remaining_value = 0;
+  $time ||= time;
 
-    my $remain_pkg = $self;
-    $remaining_value = $remain_pkg->calc_remain(
-      'time' => $time, 
-      'cust_credit_source_bill_pkg' => \@cust_credit_source_bill_pkg,
-    );
+  my $remain_pkg = $self;
+  my (@billpkgnums, @amounts, @setuprecurs);
+  
+  # we may have to walk back past some package changes to get to the 
+  # one that actually has unused time. loop until that happens, or we
+  # reach the first package in the chain.
+  while (1) {
+    my $last_bill = $remain_pkg->get('last_bill') || 0;
+    my $next_bill = $remain_pkg->get('bill') || 0;
+    if ( $last_bill > 0         # the package has been billed
+        and $next_bill > 0      # the package has a next bill date
+        and $next_bill >= $time # which is in the future
+    ) {
+
+      # Find actual charges for the period ending on or after the cancel
+      # date.
+      my @charges = qsearch('cust_bill_pkg', {
+        pkgnum => $remain_pkg->pkgnum,
+        edate => {op => '>=', value => $time},
+        recur => {op => '>' , value => 0},
+      });
+
+      foreach my $cust_bill_pkg (@charges) {
+        # hack to deal with the weird behavior of edate on package
+        # cancellation
+        my $edate = $cust_bill_pkg->edate;
+        if ( $self->recur_temporality eq 'preceding' ) {
+          $edate = $self->add_freq($cust_bill_pkg->sdate);
+        }
+
+        # this will also get any package charges that are _entirely_ after
+        # the cancellation date (can happen with advance billing). in that
+        # case, use the entire recurring charge:
+        my $amount = $cust_bill_pkg->recur - $cust_bill_pkg->usage;
+
+        # but if the cancellation happens during the interval, prorate it:
+        # (XXX obey prorate_round_day here?)
+        if ( $cust_bill_pkg->sdate < $time ) {
+          $amount = $amount *
+                      ($edate - $time) / ($edate - $cust_bill_pkg->sdate);
+        }
+
+        $amount = sprintf('%.2f', $amount);
+
+        push @billpkgnums, $cust_bill_pkg->billpkgnum;
+        push @amounts,     $amount;
+        push @setuprecurs, 'recur';
+
+        warn "Crediting for $amount on package ".$remain_pkg->pkgnum."\n"
+          if $DEBUG;
 
-    # we may have to walk back past some package changes to get to the 
-    # one that actually has unused time
-    while ( $remaining_value == 0 ) {
-      if ( $remain_pkg->change_pkgnum ) {
-        $remain_pkg = FS::cust_pkg->by_key($remain_pkg->change_pkgnum);
-      } else {
-        # the package has really never been billed
-        return;
       }
-      $remaining_value = $remain_pkg->calc_remain(
-        'time' => $time, 
-        'cust_credit_source_bill_pkg' => \@cust_credit_source_bill_pkg,
-      );
+
+      last if @charges;
     }
 
-    if ( $remaining_value > 0 ) {
-      warn "Crediting for $remaining_value on package ".$self->pkgnum."\n"
-        if $DEBUG;
-      my $error = $self->cust_main->credit(
-        $remaining_value,
-        'Credit for unused time on '. $self->part_pkg->pkg,
-        'reason_type' => $reason_type,
-        'cust_credit_source_bill_pkg' => \@cust_credit_source_bill_pkg,
-      );
-      return "Error crediting customer \$$remaining_value for unused time".
-        " on ". $self->part_pkg->pkg. ": $error"
-        if $error;
-    } #if $remaining_value
-  } #if $last_bill, etc.
+    if ( my $changed_from_pkgnum = $remain_pkg->change_pkgnum ) {
+      $remain_pkg = FS::cust_pkg->by_key($changed_from_pkgnum);
+    } else {
+      # the package has really never been billed
+      return;
+    }
+  }
+
+  # keep traditional behavior here. 
+  local $@;
+  my $reason = FS::reason->new_or_existing(
+    reason  => 'Credit for unused time on '. $self->part_pkg->pkg,
+    type    => $reason_type,
+    class   => 'R',
+  );
+  if ( $@ ) {
+    return "failed to set credit reason: $@";
+  }
+
+  my $error = FS::cust_credit->credit_lineitems(
+    'billpkgnums' => \@billpkgnums,
+    'setuprecurs' => \@setuprecurs,
+    'amounts'     => \@amounts,
+    'custnum'     => $self->custnum,
+    'date'        => time,
+    'reasonnum'   => $reason->reasonnum,
+    'apply'       => 1,
+  );
+
   '';
 }
 
diff --git a/FS/t/suite/09-sales_tax_credit_change.t b/FS/t/suite/09-sales_tax_credit_change.t
new file mode 100755 (executable)
index 0000000..58d9968
--- /dev/null
@@ -0,0 +1,102 @@
+#!/usr/bin/perl
+
+=head2 DESCRIPTION
+
+Tests crediting a package for unused time when it has sales tax. See
+RT#42729.
+
+The package will be billed for $30.00 with 10% tax, then credited for 1/3
+of the billing period.
+
+Correct: The credit amount will be $11.00.
+
+=cut
+
+use strict;
+use Test::More tests => 2;
+use FS::Test;
+use Date::Parse 'str2time';
+use Date::Format 'time2str';
+use Test::MockTime qw(set_fixed_time);
+use FS::cust_main;
+use FS::cust_pkg;
+use FS::part_pkg;
+use FS::Conf;
+my $FS= FS::Test->new;
+
+# Create a package def
+my $error;
+my $part_pkg = FS::part_pkg->new({
+  pkg     => 'Tax credit test',
+  plan    => 'flat',
+  freq    => '1',
+  agentnum => 1,
+});
+my %options = (
+  'setup_fee' => 0,
+  'recur_fee' => 30.00,
+  'recur_temporality' => 'upcoming',
+  'unused_credit_cancel' => '1',
+);
+$error = $part_pkg->insert(options => \%options);
+BAIL_OUT("can't create package def: $error") if $error;
+
+# Create the customer and order a package
+my $cust = $FS->new_customer('Credit unused with taxes');
+$cust->bill_location->state('AK');
+$error = $cust->insert;
+BAIL_OUT("can't create test customer: $error") if $error;
+
+my $pkg = FS::cust_pkg->new({ pkgpart => $part_pkg->pkgpart });
+$error = $cust->order_pkg({ cust_pkg => $pkg });
+BAIL_OUT("can't create test charges: $error") if $error;
+
+# Create tax def
+my $cust_main_county = FS::cust_main_county->new({
+  'country'       => 'US',
+  'state'         => 'AK',
+  'exempt_amount' => 0.00,
+  'taxname'       => 'Test tax',
+  'tax'           => '10',
+});
+$error = $cust_main_county->insert;
+BAIL_OUT("can't create tax definitions: $error") if $error;
+
+# Bill the customer on Apr 1
+# (April because it's 30 days, and also doesn't have DST)
+set_fixed_time(str2time('2016-04-01 00:00'));
+my @return;
+$error = $cust->bill( return_bill => \@return );
+BAIL_OUT("can't bill charges: $error") if $error;
+my $cust_bill = $return[0] or BAIL_OUT("no invoice generated");
+
+# Check amount
+my ($tax_item) = grep { $_->itemdesc eq $cust_main_county->taxname }
+                $cust_bill->cust_bill_pkg;
+ok ( $tax_item && $tax_item->setup == 3.00, "Tax charged = 3.00" );
+
+# sync
+$pkg = $pkg->replace_old;
+
+# Pay the bill
+set_fixed_time(str2time('2016-04-02 00:00'));
+my $cust_pay = FS::cust_pay->new({
+  custnum => $cust->custnum,
+  invnum  => $cust_bill->invnum,
+  _date   => time,
+  paid    => $cust_bill->owed,
+  payby   => 'CASH',
+});
+$error = $cust_pay->insert;
+BAIL_OUT("can't record payment: $error") if $error;
+
+# Now cancel with 1/3 of the period left
+set_fixed_time(str2time('2016-04-21 00:00'));
+$error = $pkg->cancel();
+BAIL_OUT("can't cancel package: $error") if $error;
+
+# and find the credit
+my ($credit) = $cust->cust_credit
+  or BAIL_OUT("no credit was created");
+ok ( $credit->amount == 11.00, "Credited 1/3 of package charge with tax" )
+  or diag("is ". $credit->amount );