cleanup for package report class filtering on sales report, #24776

4 files changed, 56 insertions, 66 deletions

diff --git a/FS/FS/Report/Table.pm b/FS/FS/Report/Table.pm
index 03ee27391..1d60b6420 100644
--- a/FS/FS/Report/Table.pm
+++ b/FS/FS/Report/Table.pm
@@ -56,12 +56,7 @@ sub signups {
     push @where, "refnum = ".$opt{'refnum'};
   }
 
-  if ( $opt{'cust_classnum'} ) {
-    my $classnums = $opt{'cust_classnum'};
-    $classnums = [ $classnums ] if !ref($classnums);
-    @$classnums = grep /^\d+$/, @$classnums;
-    push @where, 'cust_main.classnum in('. join(',',@$classnums) .')';
-  }
+  push @where, $self->with_cust_classnum(%opt);
 
   $self->scalar_sql(
     "SELECT COUNT(*) FROM cust_main $join WHERE ".join(' AND ', @where)
@@ -448,12 +443,7 @@ sub cust_bill_pkg_setup {
   # yuck, false laziness
   push @where, "cust_main.refnum = ". $opt{'refnum'} if $opt{'refnum'};
 
-  if ( $opt{'cust_classnum'} ) {
-    my $classnums = $opt{'cust_classnum'};
-    $classnums = [ $classnums ] if !ref($classnums);
-    @$classnums = grep /^\d+$/, @$classnums;
-    push @where, 'cust_main.classnum in('. join(',',@$classnums) .')';
-  }
+  push @where, $self->with_cust_classnum(%opt);
 
   my $total_sql = "SELECT COALESCE(SUM(cust_bill_pkg.setup),0)
   FROM cust_bill_pkg
@@ -478,12 +468,7 @@ sub cust_bill_pkg_recur {
 
   push @where, 'cust_main.refnum = '. $opt{'refnum'} if $opt{'refnum'};
 
-  if ( $opt{'cust_classnum'} ) {
-    my $classnums = $opt{'cust_classnum'};
-    $classnums = [ $classnums ] if !ref($classnums);
-    @$classnums = grep /^\d+$/, @$classnums;
-    push @where, 'cust_main.classnum in('. join(',',@$classnums) .')';
-  }
+  push @where, $self->with_cust_classnum(%opt);
 
   # subtract all usage from the line item regardless of date
   my $item_usage;
@@ -540,12 +525,7 @@ sub cust_bill_pkg_detail {
 
   push @where, 'cust_main.refnum = '. $opt{'refnum'} if $opt{'refnum'};
 
-  if ( $opt{'cust_classnum'} ) {
-    my $classnums = $opt{'cust_classnum'};
-    $classnums = [ $classnums ] if !ref($classnums);
-    @$classnums = grep /^\d+$/, @$classnums;
-    push @where, 'cust_main.classnum in('. join(',',@$classnums) .')';
-  }
+  push @where, $self->with_cust_classnum(%opt);
 
   $agentnum ||= $opt{'agentnum'};
 
@@ -688,12 +668,8 @@ sub for_opts {
     if ( $opt{'refnum'} =~ /^(\d+)$/ ) {
       $sql .= " and refnum = $1 ";
     }
-    if ( $opt{'cust_classnum'} ) {
-      my $classnums = $opt{'cust_classnum'};
-      $classnums = [ $classnums ] if !ref($classnums);
-      @$classnums = grep /^\d+$/, @$classnums;
-      $sql .= ' and cust_main.classnum in('. join(',',@$classnums) .')'
-        if @$classnums;
+    if ( my $where = $self->with_cust_classnum(%opt) ) {
+      $sql .= " and $where";
     }
 
     $sql;
@@ -772,6 +748,19 @@ sub with_report_option {
   $comparison;
 }
 
+sub with_cust_classnum {
+  my ($self, %opt) = @_;
+  if ( $opt{'cust_classnum'} ) {
+    my $classnums = $opt{'cust_classnum'};
+    $classnums = [ $classnums ] if !ref($classnums);
+    @$classnums = grep /^\d+$/, @$classnums;
+    return 'cust_main.classnum in('. join(',',@$classnums) .')'
+      if @$classnums;
+  }
+  '';
+}
+
+
 sub scalar_sql {
   my( $self, $sql ) = ( shift, shift );
   my $sth = dbh->prepare($sql) or die dbh->errstr;
diff --git a/httemplate/graph/cust_bill_pkg.cgi b/httemplate/graph/cust_bill_pkg.cgi
index 01d309df8..9660e5b88 100644
--- a/httemplate/graph/cust_bill_pkg.cgi
+++ b/httemplate/graph/cust_bill_pkg.cgi
@@ -97,10 +97,10 @@ if ( $cgi->param('class_mode') eq 'report' ) {
   $value_col = 'classnum';
 }
 
-my @classnums = grep /^\d+$/, $cgi->param($value_col);
+my @classnums = grep /^\d+$/, $cgi->param($class_param);
 my @classnames = map { if ( $_ ) {
                          my $class = qsearchs($class_table, {$value_col=>$_} );
-                         $class->classname;
+                         $class->$name_col;
                        } else {
                          '(empty class)';
                        }
@@ -111,8 +111,6 @@ $bottom_link .= "$class_param=$_;" foreach @classnums;
 
 if ( $cgi->param('class_agg_break') eq 'aggregate' ) {
 
-  $link .= ";$class_param=$_" foreach @classnums;
-
   $title .= ' '. join(', ', @classnames)
     unless scalar(@classnames) > scalar(qsearch($class_table,{'disabled'=>''}));
                                  #not efficient for lots of package classes
@@ -291,22 +289,22 @@ foreach my $agent ( $all_agent || $sel_agent || qsearch('agent', { 'disabled' =>
     my $row_link = "$link;".
                    "agentnum=$row_agentnum;".
                    "distribute=$distribute;".
-                   "charges=$component";
+                   "charges=$component;";
     
     # package class filters
     if ( $cgi->param('class_agg_break') eq 'aggregate' ) {
       push @row_params, $class_param => \@classnums;
-      $row_link .= ";$class_param=".$_ foreach @classnums;
+      $row_link .= "$class_param=$_;" foreach @classnums;
     }
 
     # refnum filters
     if ( $sel_part_referral ) {
       push @row_params, 'refnum' => $sel_part_referral->refnum;
-      $row_link .= ";refnum=".$sel_part_referral->refnum;
+      $row_link .= "refnum=;".$sel_part_referral->refnum;
     }
 
     # customer class filters
-    $row_link .= ";cust_classnum=$_" foreach @cust_classnums;
+    $row_link .= "cust_classnum=$_;" foreach @cust_classnums;
 
     push @items, 'cust_bill_pkg';
     push @labels, mt('[_1] - Subtotal', $agent->agent);
diff --git a/httemplate/graph/elements/report.html b/httemplate/graph/elements/report.html
index b7073db31..b3ba9ee22 100644
--- a/httemplate/graph/elements/report.html
+++ b/httemplate/graph/elements/report.html
@@ -50,19 +50,19 @@ any delimiter and linked from the elements in @data.
 <% $csv->string %>
 %
 %   my @bottom_total = ();
+%   my $row = 0;
 %   foreach ( @items ) {
 %
 %     my $col = 0;
-%     my $total = 0;
-%     $csv->combine(
-%       shift( @row_labels ),
-%       map { $total += $_; $bottom_total[$col++] += $_; sprintf($sprintf, $_); }
-%         ( @{ shift( @data ) } ),
-%       ( $opt{'nototal'} ? () : sprintf($sprintf, $total) ),
-%     );
-%     unless ( $opt{'nototal'} ) { 
-%       $bottom_total[$col++] += $total; 
-%     } 
+%     my @row = map { sprintf($sprintf, $_) } @{ shift(@data) };
+%     my $total = sum(@row);
+%     push @row, sprintf($sprintf, $total) unless $opt{'nototal'};
+%     unless ($opt{'no_graph'}[$row]) {
+%       foreach (@row) {
+%         $bottom_total[$col++] += $_;
+%       }
+%     }
+%     $csv->combine(shift(@row_labels), @row);
 <% $csv->string %>
 %
 %   }
@@ -92,35 +92,35 @@ any delimiter and linked from the elements in @data.
 %
 %   my $worksheet = $workbook->add_worksheet(substr($opt{'title'},0,31));
 %
-%   my($r,$c) = (0,0);
+%   my($row,$col) = (0,0);
 %
 %   foreach ('', @col_labels, ($opt{'nototal'} ? () : 'Total') ) {
 %     my $header = $_;
-%     $worksheet->write($r, $c++, $header)
+%     $worksheet->write($row, $col++, $header)
 %   }
 %
 %   my @bottom_total = ();
 %   foreach ( @items ) {
-%     $r++;
-%     $c = 0;
+%     $row++;
+%     $col = 0;
 %     my $total = 0;
-%     $worksheet->write( $r, $c++, shift( @row_labels ) );
+%     $worksheet->write( $row, $col++, shift( @row_labels ) );
 %     foreach ( @{ shift( @data ) } ) {
 %       $total += $_;
-%       $bottom_total[$c-1] += $_;
-%       $worksheet->write($r, $c++,  sprintf($sprintf, $_) );
+%       $bottom_total[$col-1] += $_ unless $opt{no_graph}[$row];
+%       $worksheet->write($row, $col++,  sprintf($sprintf, $_) );
 %     }
-%     unless ( $opt{'nototal'} ) { 
-%       $bottom_total[$c-1] += $total; 
-%       $worksheet->write($r, $c++,  sprintf($sprintf, $total) );
+%     if ( !$opt{'nototal'} ) {
+%       $bottom_total[$col-1] += $total unless $opt{no_graph}[$row]; 
+%       $worksheet->write($row, $col++,  sprintf($sprintf, $total) );
 %     } 
 %   }
 % 
-%   $c = 0;
+%   $col = 0;
 %   if ( $opt{'bottom_total'} ) {
-%     $r++;
-%     $worksheet->write($r, $c++, 'Total');
-%     $worksheet->write($r, $c++, sprintf($sprintf, $_)) foreach @bottom_total;
+%     $row++;
+%     $worksheet->write($row, $col++, 'Total');
+%     $worksheet->write($row, $col++, sprintf($sprintf, $_)) foreach @bottom_total;
 %   } 
 %   
 %   $workbook->close();# or die "Error creating .xls file: $!";
@@ -207,7 +207,7 @@ any delimiter and linked from the elements in @data.
             Download full results<BR>
             as <A HREF="<% "$myself;_type=xls" %>">Excel spreadsheet</A><BR>
             as <A HREF="<% "$myself;_type=csv" %>">CSV file</A></P>
-% } 
+% }
 %
 </P>
 %# indexed by item, then by entry (the element indices of @{$data[$i]}).
@@ -227,6 +227,7 @@ any delimiter and linked from the elements in @data.
 
 % # i for item, e for entry
 % my $i = 1;
+% my @bottom_total = map {0} @col_labels;
 % foreach my $row ( @items ) {
 % #make a style
 %   my $color = shift @{ $opt{'colors'} };
@@ -243,11 +244,13 @@ any delimiter and linked from the elements in @data.
 %   if ( ! $opt{'nototal'} ) {
 %     push @$data_row, sum(@$data_row);
 %   }
+%   my $e = 0;
 %   foreach ( @$data_row ) {
 %     my $entry = $_;
 %     $entry = $money_char . sprintf($sprintf, $entry);
 %     $entry = $link_prefix . shift(@$links) . "\">$entry</A>" if $link_prefix;
 %     push @{$cell[$i]}, $entry;
+%     $bottom_total[$e++] += $_ unless $opt{no_graph}[$i-1];
 %   }
 %   $i++;
 % }
@@ -260,7 +263,7 @@ any delimiter and linked from the elements in @data.
 %   $link_prefix = '<A CLASS="cell" HREF="'.$link_prefix if $link_prefix;
 %   $cell[$i] = [ emt('Total') ];
 %   for (my $e = 0; $e < $num_entries + 1; $e++) {
-%     my $entry = sum(map { $_->[$e] } @data);
+%     my $entry = $bottom_total[$e];
 %     $entry = $money_char . sprintf($sprintf, $entry);
 %     $entry = $link_prefix . shift(@$links) . "\">$entry</A>" if $link_prefix;
 %     push @{$cell[$i]}, $entry;
diff --git a/httemplate/graph/report_cust_bill_pkg.html b/httemplate/graph/report_cust_bill_pkg.html
index c9e256778..d43ce7b64 100644
--- a/httemplate/graph/report_cust_bill_pkg.html
+++ b/httemplate/graph/report_cust_bill_pkg.html
@@ -119,10 +119,10 @@ window.onload = class_mode_changed;
         </TD>
 
         <TD>
-          <INPUT TYPE="radio" NAME="class_agg_break" ID="class_agg_break_aggregate" VALUE="aggregate" CHECKED>
+          <INPUT TYPE="radio" NAME="class_agg_break" ID="class_agg_break_aggregate" VALUE="aggregate" onchange="enable_agent_totals(this)" CHECKED>
           <% emt('Aggregate') %>
           <BR>
-          <INPUT TYPE="radio" NAME="class_agg_break" VALUE="breakdown">
+          <INPUT TYPE="radio" NAME="class_agg_break" ID="class_agg_break_breakdown" VALUE="breakdown" onchange="enable_agent_totals(this)">
           <% emt('Breakdown') %>
         </TD>