RT#25563: Better handling of commissions which do not have rates configured [more error messages]

7 files changed, 32 insertions, 14 deletions

diff --git a/FS/FS/part_event/Action/Mixin/credit_bill.pm b/FS/FS/part_event/Action/Mixin/credit_bill.pm
index 5a26d2e..91fa21f 100644
--- a/FS/FS/part_event/Action/Mixin/credit_bill.pm
+++ b/FS/FS/part_event/Action/Mixin/credit_bill.pm
@@ -107,9 +107,12 @@ sub _calc_credit {
     # don't multiply by quantity here; it's already included
   }
 
-  $$warnref .= $warning if ref($warnref);
+  if ($charge < 0) { # e.g. prorate
+    $charge = 0;
+    $warning .= 'Negative charge set to zero ';
+  }
 
-  $charge = 0 if $charge < 0; # e.g. prorate
+  $$warnref .= $warning if ref($warnref);
   return ($percent * $charge / 100);
 }
 
diff --git a/FS/FS/part_event/Action/Mixin/credit_flat.pm b/FS/FS/part_event/Action/Mixin/credit_flat.pm
index 374cf5d..c35d5d8 100644
--- a/FS/FS/part_event/Action/Mixin/credit_flat.pm
+++ b/FS/FS/part_event/Action/Mixin/credit_flat.pm
@@ -19,7 +19,10 @@ sub option_fields {
 
 sub _calc_credit {
   my $self = shift;
-  $self->option('amount');
+  my $warnref = $_[2]; #other input not used by credit_flat
+  my $warning = $self->option('amount') ? '' : 'Amount set to zero ';
+  $$warnref .= $warning if ref($warnref);
+  return $self->option('amount');
 }
 
 1;
diff --git a/FS/FS/part_event/Action/Mixin/credit_pkg.pm b/FS/FS/part_event/Action/Mixin/credit_pkg.pm
index 400ece9..2842218 100644
--- a/FS/FS/part_event/Action/Mixin/credit_pkg.pm
+++ b/FS/FS/part_event/Action/Mixin/credit_pkg.pm
@@ -52,10 +52,14 @@ sub option_fields {
 # arguments:
 # 1. cust_pkg
 # 2. recipient of the credit (passed through to _calc_credit_percent)
+# 3. optional scalar reference for recording a warning message
 
 sub _calc_credit {
   my $self = shift;
   my $cust_pkg = shift;
+  my $who = shift;
+  my $warnref = shift;
+  my $warning = '';
 
   my $cust_main = $self->cust_main($cust_pkg);
 
@@ -75,15 +79,19 @@ sub _calc_credit {
 
   my $percent;
   if ( $self->can('_calc_credit_percent') ) {
-    $percent = $self->_calc_credit_percent($cust_pkg, @_);
+    $percent = $self->_calc_credit_percent($cust_pkg, $who) || 0;
+    $warning = 'Percent calculated to zero ' unless $percent+0;
   } else {
     $percent = $self->option('percent') || 0;
+    $warning = 'Percent set to zero ' unless $percent+0;
   }
 
   my @arg = ($what eq 'setup_cost') ? () : ($cust_pkg);
+  my $charge = $part_pkg->$what(@arg) || 0;
+  $warning .= "$what is zero" unless $charge+0;
 
-  sprintf('%.2f', $part_pkg->$what(@arg) * $percent / 100 );
-
+  $$warnref .= $warning if ref($warnref);
+  return sprintf('%.2f', $charge * $percent / 100 );
 }
 
 1;
diff --git a/FS/FS/part_event/Action/pkg_agent_credit.pm b/FS/FS/part_event/Action/pkg_agent_credit.pm
index 65f8c27..35cf07e 100644
--- a/FS/FS/part_event/Action/pkg_agent_credit.pm
+++ b/FS/FS/part_event/Action/pkg_agent_credit.pm
@@ -19,8 +19,9 @@ sub do_action {
   my $agent_cust_main = $agent->agent_cust_main;
     #? or return "No customer record for agent ". $agent->agent;
 
-  my $amount = $self->_calc_credit($cust_pkg, $agent);
-  return '' unless $amount > 0;
+  my $warning = '';
+  my $amount = $self->_calc_credit($cust_pkg, $agent, \$warning);
+  return $warning unless $amount > 0;
 
   my $reasonnum = $self->option('reasonnum');
 
diff --git a/FS/FS/part_event/Action/pkg_employee_credit.pm b/FS/FS/part_event/Action/pkg_employee_credit.pm
index 6cbe9bc..838d175 100644
--- a/FS/FS/part_event/Action/pkg_employee_credit.pm
+++ b/FS/FS/part_event/Action/pkg_employee_credit.pm
@@ -19,8 +19,9 @@ sub do_action {
   my $employee_cust_main = $employee->user_cust_main;
     #? or return "No customer record for employee ". $employee->username;
 
-  my $amount    = $self->_calc_credit($cust_pkg, $employee);
-  return '' unless $amount > 0;
+  my $warning = '';
+  my $amount    = $self->_calc_credit($cust_pkg, $employee, \$warning);
+  return $warning unless $amount > 0;
 
   my $reasonnum = $self->option('reasonnum');
 
diff --git a/FS/FS/part_event/Action/pkg_referral_credit.pm b/FS/FS/part_event/Action/pkg_referral_credit.pm
index 9d7bbf8..a85a3a1 100644
--- a/FS/FS/part_event/Action/pkg_referral_credit.pm
+++ b/FS/FS/part_event/Action/pkg_referral_credit.pm
@@ -23,8 +23,9 @@ sub do_action {
   return 'Referring customer is cancelled'
     if $referring_cust_main->status eq 'cancelled';
 
-  my $amount    = $self->_calc_credit($cust_pkg, $referring_cust_main);
-  return '' unless $amount > 0;
+  my $warning = '';
+  my $amount    = $self->_calc_credit($cust_pkg, $referring_cust_main, \$warning);
+  return $warning unless $amount > 0;
 
   my $reasonnum = $self->option('reasonnum');
 
diff --git a/FS/FS/part_event/Action/pkg_sales_credit.pm b/FS/FS/part_event/Action/pkg_sales_credit.pm
index 3c569ca..26a6f6d 100644
--- a/FS/FS/part_event/Action/pkg_sales_credit.pm
+++ b/FS/FS/part_event/Action/pkg_sales_credit.pm
@@ -27,8 +27,9 @@ sub do_action {
   my $sales_cust_main = $sales->sales_cust_main;
     #? or return "No customer record for sales person ". $sales->salesperson;
 
-  my $amount = $self->_calc_credit($cust_pkg, $sales);
-  return '' unless $amount > 0;
+  my $warning = '';
+  my $amount = $self->_calc_credit($cust_pkg, $sales, \$warning);
+  return $warning unless $amount > 0;
 
   my $reasonnum = $self->option('reasonnum');