no need for scalar

9 files changed, 177 insertions, 91 deletions

diff --git a/FS/FS/Report/Table.pm b/FS/FS/Report/Table.pm
index 4b1ad05d6..eeb99bac5 100644
--- a/FS/FS/Report/Table.pm
+++ b/FS/FS/Report/Table.pm
@@ -753,6 +753,33 @@ sub cust_bill_pkg_taxes {
   $self->scalar_sql($total_sql);
 }
 
+#all credits applied to matching pkg line items (ie not taxes)
+
+sub cust_bill_pkg_credits {
+  my $self = shift;
+  my ($speriod, $eperiod, $agentnum, %opt) = @_;
+
+  $agentnum ||= $opt{'agentnum'};
+
+  my @where = (
+    '(cust_bill_pkg.pkgnum != 0 OR feepart IS NOT NULL)',
+    $self->with_classnum($opt{'classnum'}, $opt{'use_override'}),
+    $self->with_report_option(%opt),
+    $self->in_time_period_and_agent($speriod, $eperiod, $agentnum),
+    $self->with_refnum(%opt),
+    $self->with_cust_classnum(%opt)
+  );
+
+  my $total_sql = "SELECT COALESCE(SUM(cust_credit_bill_pkg.amount),0)
+    FROM cust_bill_pkg
+    $cust_bill_pkg_join
+    LEFT JOIN cust_credit_bill_pkg 
+      USING ( billpkgnum )
+    WHERE " . join(' AND ', grep $_, @where);
+
+  $self->scalar_sql($total_sql);
+}
+
 ##### package churn report #####
 
 =item active_pkg: The number of packages that were active at the start of 
diff --git a/FS/FS/TicketSystem/RT_External.pm b/FS/FS/TicketSystem/RT_External.pm
index c2aac2db7..20d78c8fc 100644
--- a/FS/FS/TicketSystem/RT_External.pm
+++ b/FS/FS/TicketSystem/RT_External.pm
@@ -316,22 +316,22 @@ sub href_params_new_ticket {
   my $subtype = $object->table;
   my $pkey = $object->get($object->primary_key);
 
-  my %param = (
+  my @param = (
     'Queue'       => ($cust_main->agent->ticketing_queueid || $default_queueid),
     'new-MemberOf'=> "freeside://freeside/$subtype/$pkey",
     'Requestors'  => $requestors,
   );
 
-  ( $self->baseurl.'Ticket/Create.html', %param );
+  ( $self->baseurl.'Ticket/Create.html', @param );
 }
 
 sub href_new_ticket {
   my $self = shift;
 
-  my( $base, %param ) = $self->href_params_new_ticket(@_);
+  my( $base, @param ) = $self->href_params_new_ticket(@_);
 
   my $uri = new URI $base;
-  $uri->query_form(%param);
+  $uri->query_form(@param);
   $uri;
 
 }
diff --git a/FS/FS/UI/Web.pm b/FS/FS/UI/Web.pm
index 69de5e157..d73d9cfc0 100644
--- a/FS/FS/UI/Web.pm
+++ b/FS/FS/UI/Web.pm
@@ -623,6 +623,7 @@ sub random_id {
   if (!defined $NO_RANDOM_IDS) {
     my $conf = FS::Conf->new;
     $NO_RANDOM_IDS = $conf->exists('no_random_ids') ? 1 : 0;
+    warn "TEST MODE--RANDOM ID NUMBERS DISABLED\n" if $NO_RANDOM_IDS;
   }
   if ( $NO_RANDOM_IDS ) {
     if ( $digits > 0 ) {
diff --git a/FS/FS/cdr.pm b/FS/FS/cdr.pm
index db9832a4b..cdca6fc75 100644
--- a/FS/FS/cdr.pm
+++ b/FS/FS/cdr.pm
@@ -1458,7 +1458,7 @@ as keys (for use with part_pkg::voip_cdr) and "pretty" format names as values.
 sub invoice_formats {
   map { ($_ => $export_names{$_}->{'name'}) }
     grep { $export_names{$_}->{'invoice_header'} }
-    keys %export_names;
+    sort keys %export_names;
 }
 
 =item invoice_header FORMAT
diff --git a/FS/FS/cust_main/Billing.pm b/FS/FS/cust_main/Billing.pm
index ec780e61e..014d71529 100644
--- a/FS/FS/cust_main/Billing.pm
+++ b/FS/FS/cust_main/Billing.pm
@@ -810,6 +810,7 @@ sub bill {
 }
 
 #discard bundled packages of 0 value
+# XXX we should reconsider whether we even need this
 sub _omit_zero_value_bundles {
   my @in = @_;
 
@@ -818,11 +819,20 @@ sub _omit_zero_value_bundles {
   my $discount_show_always = $conf->exists('discount-show-always');
   my $show_this = 0;
 
+  # Sort @in the same way we do during invoice rendering, so we can identify
+  # bundles.  See FS::Template_Mixin::_items_nontax.
+  @in = sort { $a->pkgnum <=> $b->pkgnum        or
+               $a->sdate  <=> $b->sdate         or
+               ($a->pkgpart_override ? 0 : -1)  or
+               ($b->pkgpart_override ? 0 : 1)   or
+               $b->hidden cmp $a->hidden        or
+               $a->pkgpart_override <=> $b->pkgpart_override
+             } @in;
+
   # this is a pack-and-deliver pattern. every time there's a cust_bill_pkg
   # _without_ pkgpart_override, that's the start of the new bundle. if there's
   # an existing bundle, and it contains a nonzero amount (or a zero amount 
   # that's displayable anyway), push all line items in the bundle.
-
   foreach my $cust_bill_pkg ( @in ) {
 
     if (scalar(@bundle) and !$cust_bill_pkg->pkgpart_override) {
diff --git a/FS/FS/discount.pm b/FS/FS/discount.pm
index 14911395b..dd6960495 100644
--- a/FS/FS/discount.pm
+++ b/FS/FS/discount.pm
@@ -198,7 +198,13 @@ sub description {
 
   ( my $months = $self->months ) =~ s/\.0+$//;
   $months =~ s/(\.\d*[1-9])0+$/$1/;
-  $desc .= " for $months months" if $months;
+  if ($months) {
+    if ($months == 1) {
+      $desc .= " for 1 month";
+    } else {
+      $desc .= " for $months months";
+    }
+  }
 
   $desc .= ', applies to setup' if $self->setup;
 
diff --git a/FS/FS/part_pkg/discount_Mixin.pm b/FS/FS/part_pkg/discount_Mixin.pm
index 31802758c..5de7d8ea5 100644
--- a/FS/FS/part_pkg/discount_Mixin.pm
+++ b/FS/FS/part_pkg/discount_Mixin.pm
@@ -28,11 +28,15 @@ sub calc_recur {
 
 =head METHODS
 
-=item calc_discount
+=item calc_discount CUST_PKG, SDATE, DETAILS_ARRAYREF, PARAM_HASHREF
 
-Takes all the arguments of calc_recur.  Calculates and returns  the amount 
-by which to reduce the recurring fee; also increments months used on the 
-discount.
+Takes all the arguments of calc_recur.  Calculates and returns the amount 
+by which to reduce the charge; also increments months used on the discount.
+
+If there is a setup fee, this will be called once with 'setup_charge' => the
+setup fee amount (and should return the discount to be applied to the setup
+charge, if any), and again without it (for the recurring fee discount). 
+PARAM_HASHREF carries over between the two invocations.
 
 =cut
 
@@ -40,9 +44,9 @@ sub calc_discount {
   my($self, $cust_pkg, $sdate, $details, $param ) = @_;
   my $conf = new FS::Conf;
 
-  my $br = $self->base_recur_permonth($cust_pkg, $sdate);
-  $br += $param->{'override_charges'} if $param->{'override_charges'};
- 
+  my $br = $self->base_recur($cust_pkg, $sdate);
+  $br += $param->{'override_charges'} * ($cust_pkg->part_pkg->freq || 0) if $param->{'override_charges'};
+
   my $tot_discount = 0;
   #UI enforces just 1 for now, will need ordering when they can be stacked
 
@@ -80,52 +84,125 @@ sub calc_discount {
     my $discount_left;
     my $discount = $cust_pkg_discount->discount;
     #UI enforces one or the other (for now?  probably for good)
+    # $chg_months: the number of months we are charging recur for
+    # $months: $chg_months or the months left on the discount, whchever is less
+
+    my $chg_months = $cust_pkg->part_pkg->freq || 1;
+    if ( defined($param->{'months'}) ) { # then override
+      $chg_months = $param->{'months'};
+    }
+
+    my $months = $chg_months;
+    if ( $discount->months ) {
+      $months = min( $chg_months,
+                     $discount->months - $cust_pkg_discount->months_used );
+    }
+
+    # $amount is now the (estimated) discount amount on the recurring charge.
+    # if it's a percent discount, that's base recur * percentage.
+
     my $amount = 0;
-    $amount += $discount->amount
-        if $cust_pkg->pkgpart == $param->{'real_pkgpart'};
-    $amount += sprintf('%.2f', $discount->percent * $br / 100 );
-    my $chg_months = defined($param->{'months'}) ?
-                      $param->{'months'} :
-                      $cust_pkg->part_pkg->freq;
-
-    my $months = $discount->months
-    ? min( $chg_months,
-      $discount->months - $cust_pkg_discount->months_used )
-    : $chg_months;
 
     if (defined $param->{'setup_charge'}) {
+
+        # we are calculating the setup discount.
+        # if this discount doesn't apply to setup fees, skip it.
+        # if it's a percent discount, set $amount = percent * setup_charge.
+        # if it's a flat amount discount for one month:
+        # - if the discount amount > setup_charge, then set it to setup_charge,
+        #   and set 'discount_left_recur' to the difference.
+        # - otherwise set it to just the discount amount.
+        # 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 = sprintf('%.2f', $discount->percent * $param->{'setup_charge'} / 100 );
-            $months = 1;
-        } elsif ( $discount->amount > 0 && $discount->months == 1) {
-            $discount_left = $param->{'setup_charge'} - $discount->amount;
-            $amount = $param->{'setup_charge'} if $discount_left < 0;
-            $amount = $discount->amount if $discount_left >= 0;
-            $months = 1;
-                
+            $amount = $discount->percent * $param->{'setup_charge'} / 100;
+        } elsif ( $discount->amount > 0 && ($discount->months || 0) == 1) {
+            # 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} = 
-                0 - $discount_left if $discount_left < 0;
+            $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; 
         }
-    } elsif ( defined $param->{'discount_left_recur'}{$discount->discountnum}
-              && $param->{'discount_left_recur'}{$discount->discountnum} > 0
-            ) {
-        # use up transferred remainder of discount from setup
+
+    } else {
+      
+      # we are calculating a recurring fee discount. estimate the recurring
+      # fee:
+      # XXX it would be more accurate for calc_recur to just _tell us_ what
+      # it's going to charge
+
+      my $recur_charge = $br * ($cust_pkg->quantity || 1) * $chg_months / $self->freq;
+      # round this, because the real recur charge is rounded
+      $recur_charge = sprintf('%.2f', $recur_charge);
+
+      # if it's a percentage discount, calculate it based on that estimate.
+      # otherwise use the flat amount.
+      
+      if ( $discount->percent > 0 ) {
+        $amount = $recur_charge * $discount->percent / 100;
+      } elsif ( $discount->amount > 0
+                and $cust_pkg->pkgpart == $param->{'real_pkgpart'} ) {
+        $amount = $discount->amount * $months;
+      }
+
+      if ( exists $param->{'discount_left_recur'}{$discount->discountnum} ) {
+        # there is a discount_left_recur entry for this discountnum, so this
+        # is the second (recur) pass on the discount.  use up transferred
+        # remainder of discount from setup.
+        #
+        # note that discount_left_recur can now be zero.
         $amount = $param->{'discount_left_recur'}{$discount->discountnum};
         $param->{'discount_left_recur'}{$discount->discountnum} = 0;
-        $months = 1;
-    } elsif (    $discount->setup
-              && $discount->months == 1
-              && $discount->amount > 0
-            ) {
-        next;
-    }
+        $months = 1; # XXX really? not $chg_months?
+      }
+      #elsif (    $discount->setup
+      #          && ($discount->months || 0) == 1
+      #          && $discount->amount > 0
+      #        ) {
+      #    next;
+      #
+      #    RT #11512: bugfix to prevent applying flat discount to both setup
+      #    and recur. The original implementation ignored discount_left_recur
+      #    if it was zero, so if the setup fee used up the entire flat 
+      #    discount, the recurring charge would get to use the entire flat
+      #    discount also. This bugfix was a kludge. Instead, we now allow
+      #    discount_left_recur to be zero in that case, and then the available
+      #    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
+      # (prorate_defer_bill): the "discount_left_setup" amount will
+      # be subtracted in _make_lines.
+      if ( $discount->setup && $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;
+        }
+      }
 
-    if ( ! defined $param->{'setup_charge'} ) {
+      # cap the discount amount at the recur charge
+      $amount = min($amount, $recur_charge);
+
+      # if this is the base pkgpart, schedule increment_months_used to run at
+      # the end of billing. (addon packages haven't been calculated yet, so
+      # don't let the discount expire during the billing process. RT#17045.)
       if ( $cust_pkg->pkgpart == $param->{'real_pkgpart'} ) {
         push @{ $param->{precommit_hooks} }, sub {
           my $error = $cust_pkg_discount->increment_months_used($months);
@@ -133,63 +210,22 @@ sub calc_discount {
         };
       }
 
-      $amount = min($amount, $br);
-      $amount *= $months;
     }
 
     $amount = sprintf('%.2f', $amount + 0.00000001 ); #so 1.005 rounds to 1.01
 
     next unless $amount > 0;
 
-    # transfer remainder of discount, if any, to setup
-    if ( $discount->setup && $discount->amount > 0
-        && (!$discount->months || $discount->months != 1)
-        && !defined $param->{'setup_charge'}
-       )
-    {
-      $discount_left = $br - $amount;
-      if ( $discount_left < 0 ) {
-        $amount = $br;
-        $param->{'discount_left_setup'}{$discount->discountnum} = 
-          0 - $discount_left;
-      }
-    }
-
     #record details in cust_bill_pkg_discount
     my $cust_bill_pkg_discount = new FS::cust_bill_pkg_discount {
       'pkgdiscountnum' => $cust_pkg_discount->pkgdiscountnum,
       'amount'         => $amount,
       'months'         => $months,
+      # XXX should have a 'setuprecur'
     };
     push @{ $param->{'discounts'} }, $cust_bill_pkg_discount;
     $tot_discount += $amount;
 
-    #add details on discount to invoice
-    # no longer! this is now done during rendering based on the existence
-    # of the cust_bill_pkg_discount record
-    #
-    #my $money_char = $conf->config('money_char') || '$';
-    #$months = sprintf('%.2f', $months) if $months =~ /\./;
-
-    #my $d = 'Includes ';
-    #my $format;
-
-    #if ( $months eq '1' ) {
-    #  $d .= "discount of $money_char$amount";
-    #  $d .= " each" if $cust_pkg->quantity > 1;
-    #  $format = 'Undiscounted amount: %s%.2f';
-    #} else {
-    #  $d .= 'setup ' if defined $param->{'setup_charge'};
-    #  $d .= 'discount of '. $discount->description_short;
-    #  $d .= " for $months months"
-    #    unless defined $param->{'setup_charge'};
-    #  $d .= ": $money_char$amount" if $discount->percent;
-    #  $format = 'Undiscounted monthly amount: %s%.2f';
-    #}
-
-    #push @$details, $d;
-    #push @$details, sprintf( $format, $money_char, $br );
-
   }
 
   sprintf('%.2f', $tot_discount);
diff --git a/FS/FS/part_pkg/prorate_calendar.pm b/FS/FS/part_pkg/prorate_calendar.pm
index 83a80f5d0..c50cae0d7 100644
--- a/FS/FS/part_pkg/prorate_calendar.pm
+++ b/FS/FS/part_pkg/prorate_calendar.pm
@@ -36,7 +36,7 @@ use base 'FS::part_pkg::flat';
   },
   'fieldorder' => [ 'cutoff_day', 'prorate_defer_bill', 'prorate_round_day', 'prorate_verbose' ],
   'freq' => 'm',
-  'weight' => 20,
+  'weight' => 23,
 );
 
 my %freq_max_days = ( # the length of the shortest period of each cycle type
diff --git a/FS/FS/svc_Tower_Mixin.pm b/FS/FS/svc_Tower_Mixin.pm
index 3da07c1cd..dab096ddc 100644
--- a/FS/FS/svc_Tower_Mixin.pm
+++ b/FS/FS/svc_Tower_Mixin.pm
@@ -37,7 +37,13 @@ sub tower_sector_sql {
       my $in = join(',', map { /^(\d+)$/ ? $1 : () } @$value);
       my @orwhere;
       push @orwhere, "tower_sector.$field IN ($in)" if $in;
-      push @orwhere, "tower_sector.$field IS NULL" if grep /^none$/, @$value;
+      if ( grep /^none$/, @$value ) {
+        # then allow this field to be null
+        push @orwhere, "tower_sector.$field IS NULL";
+        # and if this field is the sector, also allow the default sector
+        # on the tower
+        push @orwhere, "sectorname = '_default'" if $field eq 'sectornum';
+      }
       push @where, '( '.join(' OR ', @orwhere).' )';
     }
     elsif ( $value =~ /^(\d+)$/ ) {