separate setup and recur discounts, #14092

9 files changed, 177 insertions, 78 deletions

diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm
index 479ab1081..ceb347d8e 100644
--- a/FS/FS/Schema.pm
+++ b/FS/FS/Schema.pm
@@ -2906,6 +2906,7 @@ sub tables_hashref {
         'otaker',        'varchar', 'NULL',    32, '', '', 
         'usernum',           'int', 'NULL',    '', '', '',
         'disabled',         'char', 'NULL',     1, '', '', 
+        'setuprecur',       'char', 'NULL',     5, '', '',
       ],
       'primary_key'  => 'pkgdiscountnum',
       'unique'       => [],
diff --git a/FS/FS/Template_Mixin.pm b/FS/FS/Template_Mixin.pm
index 1a3217c44..ffaef9707 100644
--- a/FS/FS/Template_Mixin.pm
+++ b/FS/FS/Template_Mixin.pm
@@ -3050,6 +3050,9 @@ sub _items_cust_bill_pkg {
     # if the current line item is waiting to go out, and the one we're about
     # to start is not bundled, then push out the current one and start a new
     # one.
+    if ( $d ) {
+      $d->{amount} = $d->{setup_amount} + $d->{recur_amount};
+    }
     foreach ( $s, $r, ($opt{skip_usage} ? () : $u ), $d ) {
       if ( $_ && !$cust_bill_pkg->hidden ) {
         $_->{amount}      = sprintf( "%.2f", $_->{amount} );
@@ -3485,7 +3488,8 @@ sub _items_cust_bill_pkg {
           # $item_discount->{amount} is negative
 
           if ( $d and $cust_bill_pkg->hidden ) {
-            $d->{amount}      += $item_discount->{amount};
+            $d->{setup_amount} += $item_discount->{setup_amount};
+            $d->{recur_amount} += $item_discount->{recur_amount};
           } else {
             $d = $item_discount;
             $_ = &{$escape_function}($_) foreach @{ $d->{ext_description} };
@@ -3493,27 +3497,9 @@ sub _items_cust_bill_pkg {
 
           # update the active line (before the discount) to show the 
           # original price (whether this is a hidden line or not)
-          #
-          # quotation discounts keep track of setup and recur; invoice 
-          # discounts currently don't
-          if ( exists $item_discount->{setup_amount} ) {
-
-            $s->{amount} -= $item_discount->{setup_amount} if $s;
-            $r->{amount} -= $item_discount->{recur_amount} if $r;
 
-          } else {
-
-            # $active_line is the line item hashref for the line that will
-            # show the original price
-            # (use the recur or single line for the package, unless we're 
-            # showing a setup line for a package with no recurring fee)
-            my $active_line = $r;
-            if ( $type eq 'S' ) {
-              $active_line = $s;
-            }
-            $active_line->{amount} -= $item_discount->{amount};
-
-          }
+          $s->{amount} -= $item_discount->{setup_amount} if $s;
+          $r->{amount} -= $item_discount->{recur_amount} if $r;
 
         } # if there are any discounts
       } # if this is an appropriate place to show discounts
@@ -3522,6 +3508,11 @@ sub _items_cust_bill_pkg {
 
   }
 
+  # discount amount is internally split up
+  if ( $d ) {
+    $d->{amount} = $d->{setup_amount} + $d->{recur_amount};
+  }
+
   foreach ( $s, $r, ($opt{skip_usage} ? () : $u ), $d ) {
     if ( $_  ) {
       $_->{amount}      = sprintf( "%.2f", $_->{amount} ),
diff --git a/FS/FS/cust_bill_pkg.pm b/FS/FS/cust_bill_pkg.pm
index 178042666..5861ee47f 100644
--- a/FS/FS/cust_bill_pkg.pm
+++ b/FS/FS/cust_bill_pkg.pm
@@ -820,6 +820,8 @@ quantity.
 
 sub _item_discount {
   my $self = shift;
+  my %options = @_;
+
   my @pkg_discounts = $self->pkg_discount;
   return if @pkg_discounts == 0;
   # special case: if there are old "discount details" on this line item, don't
@@ -832,7 +834,8 @@ sub _item_discount {
   my $d = {
     _is_discount    => 1,
     description     => $self->mt('Discount'),
-    amount          => 0,
+    setup_amount    => 0,
+    recur_amount    => 0,
     ext_description => \@ext,
     pkgpart         => $self->pkgpart,
     feepart         => $self->feepart,
@@ -840,9 +843,11 @@ sub _item_discount {
   };
   foreach my $pkg_discount (@pkg_discounts) {
     push @ext, $pkg_discount->description;
-    $d->{amount} -= $pkg_discount->amount;
+    my $setuprecur = $pkg_discount->cust_pkg_discount->setuprecur;
+    $d->{$setuprecur.'_amount'} -= $pkg_discount->amount;
   } 
-  $d->{amount} *= $self->quantity || 1;
+  $d->{setup_amount} *= $self->quantity || 1; # ??
+  $d->{recur_amount} *= $self->quantity || 1; # ??
   
   return $d;
 }
diff --git a/FS/FS/cust_bill_pkg_discount.pm b/FS/FS/cust_bill_pkg_discount.pm
index 9e64d2076..616657a4f 100644
--- a/FS/FS/cust_bill_pkg_discount.pm
+++ b/FS/FS/cust_bill_pkg_discount.pm
@@ -135,10 +135,36 @@ Returns a string describing the discount (for use on an invoice).
 sub description {
   my $self = shift;
   my $discount = $self->cust_pkg_discount->discount;
+
+  if ( $self->months == 0 ) {
+    # then this is a setup discount
+    my $desc = $discount->name;
+    if ( $desc ) {
+      $desc .= ': ';
+    } else {
+      $desc = $self->mt('Setup discount of ');
+    }
+    if ( (my $percent = $discount->percent) > 0 ) {
+      $percent = sprintf('%.1f', $percent) if $percent > int($percent);
+      $percent =~ s/\.0+$//;
+      $desc .= $percent . '%';
+    } else {
+      # note "$self->amount", not $discount->amount. if a flat discount
+      # is applied to the setup fee, show the amount actually discounted.
+      # we might do this for all types of discounts.
+      my $money_char = FS::Conf->new->config('money_char') || '$';
+      $desc .= $money_char . sprintf('%.2f', $self->amount);
+    }
+  
+    # don't show "/month", months remaining or used, etc., as for setup
+    # discounts it doesn't matter.
+    return $desc;
+  }
+
   my $desc = $discount->description_short;
   $desc .= $self->mt(' each') if $self->cust_bill_pkg->quantity > 1;
 
-  if ($discount->months) {
+  if ( $discount->months and $self->months > 0 ) {
     # calculate months remaining on this cust_pkg_discount after this invoice
     my $date = $self->cust_bill_pkg->cust_bill->_date;
     my $used = FS::Record->scalar_sql(
@@ -152,7 +178,7 @@ sub description {
     $used ||= 0;
     my $remaining = sprintf('%.2f', $discount->months - $used);
     $desc .= $self->mt(' for [quant,_1,month] ([quant,_2,month] remaining)',
-              $self->months,
+              sprintf('%.2f', $self->months),
               $remaining
              );
   }
diff --git a/FS/FS/cust_main/Packages.pm b/FS/FS/cust_main/Packages.pm
index c147e551c..ead97f2c3 100644
--- a/FS/FS/cust_main/Packages.pm
+++ b/FS/FS/cust_main/Packages.pm
@@ -197,7 +197,7 @@ sub order_pkg {
         map { $_ => $cust_pkg->$_() }
           qw( pkgbatch
               start_date order_date expire adjourn contract_end
-              refnum discountnum waive_setup
+              refnum setup_discountnum recur_discountnum waive_setup
             )
     });
     $error = $self->order_pkg('cust_pkg'    => $pkg,
diff --git a/FS/FS/cust_pkg.pm b/FS/FS/cust_pkg.pm
index 279205b19..d7419078b 100644
--- a/FS/FS/cust_pkg.pm
+++ b/FS/FS/cust_pkg.pm
@@ -425,7 +425,7 @@ sub insert {
     }
   }
 
-  if ( $self->discountnum ) {
+  if ( $self->setup_discountnum || $self->recur_discountnum ) {
     my $error = $self->insert_discount();
     if ( $error ) {
       $dbh->rollback if $oldAutoCommit;
@@ -4318,13 +4318,10 @@ sub insert_reason {
 Associates this package with a discount (see L<FS::cust_pkg_discount>, possibly
 inserting a new discount on the fly (see L<FS::discount>).
 
-Available options are:
-
-=over 4
-
-=item discountnum
-
-=back
+This will look at the cust_pkg for a pseudo-field named "setup_discountnum",
+and if present, will create a setup discount. If the discountnum is -1,
+a new discount definition will be inserted using the value in
+"setup_discountnum_amount" or "setup_discountnum_percent". Likewise for recur.
 
 If there is an error, returns the error, otherwise returns false.
 
@@ -4334,21 +4331,29 @@ sub insert_discount {
   #my ($self, %options) = @_;
   my $self = shift;
 
-  my $cust_pkg_discount = new FS::cust_pkg_discount {
-    'pkgnum'      => $self->pkgnum,
-    'discountnum' => $self->discountnum,
-    'months_used' => 0,
-    'end_date'    => '', #XXX
-    #for the create a new discount case
-    '_type'       => $self->discountnum__type,
-    'amount'      => $self->discountnum_amount,
-    'percent'     => $self->discountnum_percent,
-    'months'      => $self->discountnum_months,
-    'setup'      => $self->discountnum_setup,
-    #'disabled'    => $self->discountnum_disabled,
-  };
+  foreach my $x (qw(setup recur)) {
+    if ( my $discountnum = $self->get("${x}_discountnum") ) {
+      my $cust_pkg_discount = FS::cust_pkg_discount->new( {
+        'pkgnum'      => $self->pkgnum,
+        'discountnum' => $discountnum,
+        'setuprecur'  => $x,
+        'months_used' => 0,
+        'end_date'    => '', #XXX
+        #for the create a new discount case
+        'amount'      => $self->get("${x}_discountnum_amount"),
+        'percent'     => $self->get("${x}_discountnum_percent"),
+        'months'      => $self->get("${x}_discountnum_months"),
+      } );
+      if ( $x eq 'setup' ) {
+        $cust_pkg_discount->setup('Y');
+        $cust_pkg_discount->months('');
+      }
+      my $error = $cust_pkg_discount->insert;
+      return $error if $error;
+    }
+  }
 
-  $cust_pkg_discount->insert;
+  '';
 }
 
 =item set_usage USAGE_VALUE_HASHREF 
diff --git a/FS/FS/cust_pkg_discount.pm b/FS/FS/cust_pkg_discount.pm
index 5d0f85b5e..aa8981621 100644
--- a/FS/FS/cust_pkg_discount.pm
+++ b/FS/FS/cust_pkg_discount.pm
@@ -59,6 +59,9 @@ end_date
 
 order taker, see L<FS::access_user>
 
+=item setuprecur
+
+whether this discount applies to setup fees or recurring fees
 
 =back
 
@@ -125,11 +128,29 @@ sub check {
     || $self->ut_alphan('otaker')
     || $self->ut_numbern('usernum')
     || $self->ut_enum('disabled', [ '', 'Y' ] )
+    || $self->ut_enum('setuprecur', [ 'setup', 'recur' ] )
   ;
   return $error if $error;
 
-  return "Discount does not apply to setup fees, and package has no recurring"
-    if ! $self->discount->setup && $self->cust_pkg->part_pkg->freq =~ /^0/;
+  my $cust_pkg = $self->cust_pkg;
+  my $discount = $self->discount;
+  if ( $self->setuprecur eq 'setup' ) {
+    if ( !$discount->setup ) {
+      # UI prevents this, and historical discounts should never have it either
+      return "Discount #".$self->discountnum." can't be applied to setup fees.";
+    } elsif ( $cust_pkg->base_setup == 0 ) {
+      # and this
+      return "Can't apply setup discount to a package with no setup fee.";
+    }
+    # else we're good. do NOT disallow applying setup discounts when the
+    # setup date is already set; upgrades use that.
+  } else {
+    if ( $self->cust_pkg->base_recur == 0 ) {
+      return "Can't apply recur discount to a package with no recurring fee.";
+    } elsif ( $cust_pkg->part_pkg->freq eq '0' ) {
+      return "Can't apply recur discount to a one-time charge.";
+    }
+  }
 
   $self->usernum($FS::CurrentUser::CurrentUser->usernum) unless $self->usernum;
 
@@ -205,6 +226,45 @@ sub status {
 sub _upgrade_data {  # class method
   my ($class, %opts) = @_;
   $class->_upgrade_otaker(%opts);
+
+  # #14092: set setuprecur field on discounts. if we get one that applies to
+  # both setup and recur, split it into two discounts.
+  my $search = FS::Cursor->new({
+      table   => 'cust_pkg_discount',
+      hashref => { setuprecur => '' }
+  });
+  while ( my $cust_pkg_discount = $search->fetch ) {
+    my $discount = $cust_pkg_discount->discount;
+    my $cust_pkg = $cust_pkg_discount->cust_pkg;
+    # 1. Does it apply to the setup fee?
+    # Yes, if: the discount applies to setup fees generally, and the package
+    # has a setup fee.
+    # No, if: the discount is a flat amount, and is not first-month only.
+    if ( $discount->setup
+        and $cust_pkg->base_setup > 0
+        and ($discount->amount == 0 or $discount->months == 1)
+       )
+    {
+      # then clone this discount into a new one
+      my $setup_discount = FS::cust_pkg_discount->new({
+          $cust_pkg_discount->hash,
+          setuprecur      => 'setup',
+          pkgdiscountnum  => ''
+      });
+      my $error = $setup_discount->insert;
+      die "$error (migrating cust_pkg_discount to setup discount)" if $error;
+    }
+    # 2. Does it apply to the recur fee?
+    # Yes, if: the package has a recur fee.
+    if ( $cust_pkg->base_recur > 0 ) {
+      # then modify this discount in place
+      $cust_pkg_discount->set('setuprecur' => 'recur');
+      my $error = $cust_pkg_discount->replace;
+      die "$error (migrating cust_pkg_discount)" if $error;
+    }
+    # not in here yet: splitting the cust_bill_pkg_discount records.
+    # (not really necessary)
+  }
 }
 
 =back
diff --git a/FS/FS/discount.pm b/FS/FS/discount.pm
index e11335741..13146a91b 100644
--- a/FS/FS/discount.pm
+++ b/FS/FS/discount.pm
@@ -119,12 +119,12 @@ sub check {
 
   if ( $self->_type eq 'Select discount type' ) {
     return 'Please select a discount type';
-  } elsif ( $self->_type eq 'Amount' ) {
-    $self->percent('0');
-    return 'Amount must be greater than 0' unless $self->amount > 0;
-  } elsif ( $self->_type eq 'Percentage' ) {
-    $self->amount('0.00');
-    return 'Percentage must be greater than 0' unless $self->percent > 0;
+  } elsif ( $self->amount > 0 ) {
+    $self->set('percent', '0');
+  } elsif ( $self->percent > 0 ) {
+    $self->set('amount', '0.00');
+  } else {
+    return "Discount amount or percentage must be > 0";
   }
 
   my $error = 
diff --git a/FS/FS/part_pkg/discount_Mixin.pm b/FS/FS/part_pkg/discount_Mixin.pm
index 5de7d8ea5..1e39f6aef 100644
--- a/FS/FS/part_pkg/discount_Mixin.pm
+++ b/FS/FS/part_pkg/discount_Mixin.pm
@@ -50,6 +50,9 @@ sub calc_discount {
   my $tot_discount = 0;
   #UI enforces just 1 for now, will need ordering when they can be stacked
 
+  # discount setup/recur splitting DOES NOT TOUCH THIS YET.
+  # we need some kind of monitoring to see who if anyone still uses term
+  # discounts.
   if ( $param->{freq_override} ) {
     # When a customer pays for more than one month at a time to receive a 
     # term discount, freq_override is set to the number of months.
@@ -80,6 +83,13 @@ sub calc_discount {
   }
 
   my @cust_pkg_discount = $cust_pkg->cust_pkg_discount_active;
+
+  if ( defined $param->{'setup_charge'} ) {
+    @cust_pkg_discount = grep { $_->setuprecur eq 'setup' } @cust_pkg_discount;
+  } else {
+    @cust_pkg_discount = grep { $_->setuprecur eq 'recur' } @cust_pkg_discount;
+  }
+    
   foreach my $cust_pkg_discount ( @cust_pkg_discount ) {
     my $discount_left;
     my $discount = $cust_pkg_discount->discount;
@@ -115,23 +125,17 @@ sub calc_discount {
         # if it's a flat amount discount for other than one month:
         # - skip the discount. unsure, leaving it alone for now.
 
-        next unless $discount->setup;
-
         $months = 0; # never count a setup discount as a month of discount
                      # (the recur discount in the same month should do it)
 
         if ( $discount->percent > 0 ) {
             $amount = $discount->percent * $param->{'setup_charge'} / 100;
-        } elsif ( $discount->amount > 0 && ($discount->months || 0) == 1) {
+        } elsif ( $discount->amount > 0 ) {
             # apply the discount amount, up to a maximum of the setup charge
             $amount = min($discount->amount, $param->{'setup_charge'});
             $discount_left = sprintf('%.2f', $discount->amount - $amount);
             # transfer remainder of discount, if any, to recur
             $param->{'discount_left_recur'}{$discount->discountnum} = $discount_left;
-        } else {
-          # I guess we don't allow multiple-month flat amount discounts to
-          # apply to setup?
-            next; 
         }
 
     } else {
@@ -180,20 +184,27 @@ sub calc_discount {
       #    recur discount is zero. 
       #}
 
-      # transfer remainder of discount, if any, to setup
-      # this is used when the recur phase wants to add a setup fee
+      # Transfer remainder of discount, if any, to setup
+      # This is used when the recur phase wants to add a setup fee
       # (prorate_defer_bill): the "discount_left_setup" amount will
-      # be subtracted in _make_lines.
-      if ( $discount->setup && $discount->amount > 0
-          && ($discount->months || 0) != 1
-         )
+      # be subtracted in _make_lines. 
+      if ( $discount->amount > 0 && ($discount->months || 0) != 1 )
       {
-        # $amount is no longer permonth at this point! correct. very good.
-        $discount_left = $amount - $recur_charge; # backward, as above
-        if ( $discount_left > 0 ) {
-          $amount = $recur_charge;
-          $param->{'discount_left_setup'}{$discount->discountnum} = 
-            0 - $discount_left;
+        # make sure there is a setup discount with this discountnum
+        # on the same package.
+        if ( qsearchs('cust_pkg_discount', {
+              pkgnum      => $cust_pkg->pkgnum,
+              discountnum => $discount->discountnum,
+              setuprecur  => 'setup'
+            }) )
+        {
+          # $amount is no longer permonth at this point! correct. very good.
+          $discount_left = $amount - $recur_charge; # backward, as above
+          if ( $discount_left > 0 ) {
+            $amount = $recur_charge;
+            $param->{'discount_left_setup'}{$discount->discountnum} = 
+              0 - $discount_left;
+          }
         }
       }
 
@@ -210,7 +221,7 @@ sub calc_discount {
         };
       }
 
-    }
+    } # else not 'setup_charge'
 
     $amount = sprintf('%.2f', $amount + 0.00000001 ); #so 1.005 rounds to 1.01
 
@@ -221,7 +232,7 @@ sub calc_discount {
       'pkgdiscountnum' => $cust_pkg_discount->pkgdiscountnum,
       'amount'         => $amount,
       'months'         => $months,
-      # XXX should have a 'setuprecur'
+      # 'setuprecur' is implied by the cust_pkg_discount link
     };
     push @{ $param->{'discounts'} }, $cust_bill_pkg_discount;
     $tot_discount += $amount;