[freeside-commits] branch master updated. 5b670255328fbe875196e16bc8dfc57771753e90
Mark Wells
mark at 420.am
Thu Jan 9 16:38:00 PST 2014
The branch, master has been updated
via 5b670255328fbe875196e16bc8dfc57771753e90 (commit)
from 87a59b1bdf236765177c27ab18390ef1317cc34c (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 5b670255328fbe875196e16bc8dfc57771753e90
Author: Mark Wells <mark at freeside.biz>
Date: Thu Jan 9 16:37:39 2014 -0800
sales report: fix filtering by report class, #25459, #24776
diff --git a/FS/FS/Report/Table.pm b/FS/FS/Report/Table.pm
index da49161..7f59384 100644
--- a/FS/FS/Report/Table.pm
+++ b/FS/FS/Report/Table.pm
@@ -436,7 +436,7 @@ sub cust_bill_pkg_setup {
my @where = (
'pkgnum != 0',
$self->with_classnum($opt{'classnum'}, $opt{'use_override'}),
- $self->with_report_option($opt{'report_optionnum'}, $opt{'use_override'}),
+ $self->with_report_option(%opt),
$self->in_time_period_and_agent($speriod, $eperiod, $agentnum),
);
@@ -463,7 +463,7 @@ sub cust_bill_pkg_recur {
my @where = (
'pkgnum != 0',
$self->with_classnum($opt{'classnum'}, $opt{'use_override'}),
- $self->with_report_option($opt{'report_optionnum'}, $opt{'use_override'}),
+ $self->with_report_option(%opt),
);
push @where, 'cust_main.refnum = '. $opt{'refnum'} if $opt{'refnum'};
@@ -532,7 +532,7 @@ sub cust_bill_pkg_detail {
push @where,
$self->with_classnum($opt{'classnum'}, $opt{'use_override'}),
$self->with_usageclass($opt{'usageclass'}),
- $self->with_report_option($opt{'report_optionnum'}, $opt{'use_override'}),
+ $self->with_report_option(%opt),
;
if ( $opt{'distribute'} ) {
@@ -708,44 +708,71 @@ sub with_usageclass {
}
sub with_report_option {
- my ($self, $num, $use_override) = @_;
- # $num can be a single number, or a comma-delimited list of numbers,
- # or an arrayref. 0 matches the empty set
- # or the word 'multiple' for all packages with more than one report class
- return '' if !defined($num);
-
- $num = join(',', @$num) if ref($num);
-
- # stringify the set of report options for each pkgpart
- my $table = $use_override ? 'override' : 'part_pkg';
- my $subselect = "
- SELECT replace(optionname, 'report_option_', '') AS num
- FROM part_pkg_option
- WHERE optionname like 'report_option_%'
- AND part_pkg_option.pkgpart = $table.pkgpart
- ORDER BY num";
-
- my $comparison;
- if ( $num eq 'multiple' ) {
- $comparison = "(SELECT COUNT(*) FROM ($subselect) AS x) > 1";
- } else {
-
- my @num = split(/\s*,\s*/, $num);
-
- #$comparison = "(SELECT COALESCE(string_agg(num, ','), '') FROM ( #Pg 9-ism
- $comparison = "(SELECT COALESCE(array_to_string(array_agg(num), ','), '')
- FROM ($subselect) AS x
- ) = '". join(',', grep $_, @num). "'";
-
- $comparison = "( $comparison OR NOT EXISTS ($subselect) )"
- if grep !$_, @num;
-
+ my ($self, %opt) = @_;
+ # %opt can contain:
+ # - report_optionnum: a comma-separated list of numbers. Zero means to
+ # include packages with _no_ report classes.
+ # - not_report_optionnum: a comma-separated list. Packages that have
+ # any of these report options will be excluded from the result.
+ # Zero does nothing.
+ # - use_override: also matches line items that are add-ons to a package
+ # matching the report class.
+ # - all_report_options: returns only packages that have ALL of the
+ # report classes listed in $num. Otherwise, will return packages that
+ # have ANY of those classes.
+
+ my @num = ref($opt{'report_optionnum'})
+ ? @{ $opt{'report_optionnum'} }
+ : split(/\s*,\s*/, $opt{'report_optionnum'});
+ my @not_num = ref($opt{'not_report_optionnum'})
+ ? @{ $opt{'not_report_optionnum'} }
+ : split(/\s*,\s*/, $opt{'not_report_optionnum'});
+ my $null;
+ $null = 1 if ( grep {$_ == 0} @num );
+ @num = grep {$_ > 0} @num;
+ @not_num = grep {$_ > 0} @not_num;
+
+ # brute force
+ my $table = $opt{'use_override'} ? 'override' : 'part_pkg';
+ my $op = ' OR ';
+ if ( $opt{'all_report_options'} ) {
+ if ( @num and $null ) {
+ return 'false'; # mutually exclusive criteria, so just bail out
+ }
+ $op = ' AND ';
}
- if ( $use_override ) {
- # then also allow the non-override package to match
- $comparison = "( $comparison OR " . $self->with_report_option($num) . ")";
+ my @where_num = map {
+ "EXISTS(SELECT 1 FROM part_pkg_option ".
+ "WHERE optionname = 'report_option_$_' ".
+ "AND part_pkg_option.pkgpart = $table.pkgpart)"
+ } @num;
+ if ( $null ) {
+ push @where_num, "NOT EXISTS(SELECT 1 FROM part_pkg_option ".
+ "WHERE optionname LIKE 'report_option_%' ".
+ "AND part_pkg_option.pkgpart = $table.pkgpart)";
+ }
+ my @where_not_num = map {
+ "NOT EXISTS(SELECT 1 FROM part_pkg_option ".
+ "WHERE optionname = 'report_option_$_' ".
+ "AND part_pkg_option.pkgpart = $table.pkgpart)"
+ } @not_num;
+
+ my @where;
+ if (@where_num) {
+ push @where, '( '.join($op, @where_num).' )';
+ }
+ if (@where_not_num) {
+ push @where, '( '.join(' AND ', @where_not_num).' )';
}
- $comparison;
+
+ return @where;
+ # this messes up totals
+ #if ( $opt{'use_override'} ) {
+ # # then also allow the non-override package to match
+ # delete $opt{'use_override'};
+ # $comparison = "( $comparison OR " . $self->with_report_option(%opt) . ")";
+ #}
+
}
sub with_cust_classnum {
diff --git a/httemplate/graph/cust_bill_pkg.cgi b/httemplate/graph/cust_bill_pkg.cgi
index 39c9722..1b31955 100644
--- a/httemplate/graph/cust_bill_pkg.cgi
+++ b/httemplate/graph/cust_bill_pkg.cgi
@@ -84,13 +84,19 @@ $bottom_link .= "cust_classnum=$_;" foreach @cust_classnums;
#started out as false lazinessish w/FS::cust_pkg::search_sql (previously search/cust_pkg.cgi), but not much left the sane now after #24776
my ($class_table, $name_col, $value_col, $class_param);
+my $all_report_options;
if ( $cgi->param('class_mode') eq 'report' ) {
$class_param = 'report_optionnum'; # CGI param name, also used in the report engine
$class_table = 'part_pkg_report_option'; # table containing classes
$name_col = 'name'; # the column of that table containing the label
$value_col = 'num'; # the column containing the class number
-} else {
+ # in 'exact' mode we want to run the query in ALL mode.
+ # in 'breakdown' mode want to run the query in ALL mode but using the
+ # power set of the classes selected.
+ $all_report_options = 1
+ unless $cgi->param('class_agg_break') eq 'aggregate';
+} else { # class_mode eq 'pkg'
$class_param = 'classnum';
$class_table = 'pkg_class';
$name_col = 'classname';
@@ -106,10 +112,12 @@ my @classnames = map { if ( $_ ) {
}
}
@classnums;
+my @not_classnums;
$bottom_link .= "$class_param=$_;" foreach @classnums;
-if ( $cgi->param('class_agg_break') eq 'aggregate' ) {
+if ( $cgi->param('class_agg_break') eq 'aggregate' or
+ $cgi->param('class_agg_break') eq 'exact' ) {
$title .= ' '. join(', ', @classnames)
unless scalar(@classnames) > scalar(qsearch($class_table,{'disabled'=>''}));
@@ -117,15 +125,28 @@ if ( $cgi->param('class_agg_break') eq 'aggregate' ) {
} elsif ( $cgi->param('class_agg_break') eq 'breakdown' ) {
- if ( $cgi->param('mode') eq 'report' ) {
- # In theory, a package can belong to any subset of the report classes,
- # so the report groups should be all the _subsets_, but for now we're
- # handling the simple case where each package belongs to one report
- # class. Packages with multiple classes will go into one bin at the
- # end.
- push @classnames, '(multiple classes)';
- push @classnums, 'multiple';
+ if ( $cgi->param('class_mode') eq 'report' ) {
+ # The new way:
+ # Actually break down all subsets of the (selected) report classes.
+ my $powerset = sub {
+ my @set = [];
+ foreach my $x (@_) {
+ @set = map { $_, [ @$_, $x ] } @set;
+ }
+ @set;
+ };
+ @classnums = $powerset->(@classnums);
+ @classnames = $powerset->(@classnames);
+ # this is pairwise complementary to @classnums, because math
+ @not_classnums = reverse(@classnums);
+warn Dumper(\@classnums, \@classnames, \@not_classnums);
+ # remove the null set
+ shift @classnums;
+ shift @classnames;
+ shift @not_classnums;
}
+ # else it's 'pkg', i.e. part_pkg.classnum, which is singular on pkgpart
+ # and much simpler
} else {
die "guru meditation #434";
@@ -185,7 +206,10 @@ foreach my $agent ( $all_agent || $sel_agent || $FS::CurrentUser::CurrentUser->a
'distribute' => $distribute,
);
- if ( $cgi->param('class_agg_break') eq 'aggregate' ) {
+ if ( $cgi->param('class_agg_break') eq 'aggregate' or
+ $cgi->param('class_agg_break') eq 'exact' ) {
+ # the only difference between 'aggregate' and 'exact' is whether
+ # we pass the 'all_report_options' flag.
foreach my $component ( @components ) {
@@ -198,14 +222,16 @@ foreach my $agent ( $all_agent || $sel_agent || $FS::CurrentUser::CurrentUser->a
my $row_agentnum = $all_agent || $agent->agentnum;
my $row_refnum = $all_part_referral || $part_referral->refnum;
- push @params, [
+ my @row_params = (
@base_params,
$class_param => \@classnums,
($all_agent ? () : ('agentnum' => $row_agentnum) ),
($all_part_referral ? () : ('refnum' => $row_refnum) ),
- 'charges' => $component,
- ];
+ 'charges' => $component,
+ );
+ # XXX this is very silly. we should cache it server-side and
+ # just put a cache identifier in the link
my $rowlink = "$link;".
($all_agent ? '' : "agentnum=$row_agentnum;").
($all_part_referral ? '' : "refnum=$row_refnum;").
@@ -213,6 +239,11 @@ foreach my $agent ( $all_agent || $sel_agent || $FS::CurrentUser::CurrentUser->a
"distribute=$distribute;".
"use_override=$use_override;charges=$component;";
$rowlink .= "$class_param=$_;" foreach @classnums;
+ if ( $all_report_options ) {
+ push @row_params, 'all_report_options', 1;
+ $rowlink .= 'all_report_options=1';
+ }
+ push @params, \@row_params;
push @links, $rowlink;
@colorbuf = @agent_colors unless @colorbuf;
@@ -223,9 +254,12 @@ foreach my $agent ( $all_agent || $sel_agent || $FS::CurrentUser::CurrentUser->a
} elsif ( $cgi->param('class_agg_break') eq 'breakdown' ) {
+ # if we're working with report options, @classnums here contains
+ # arrays of multiple classnums
for (my $i = 0; $i < scalar @classnums; $i++) {
- my $row_classnum = $classnums[$i];
- my $row_classname = $classnames[$i];
+ my $row_classnum = join(',', @{ $classnums[$i] });
+ my $row_classname = join(', ', @{ $classnames[$i] });
+ my $not_row_classnum = join(',', @{ $not_classnums[$i] });
foreach my $component ( @components ) {
push @items, 'cust_bill_pkg';
@@ -237,21 +271,30 @@ foreach my $agent ( $all_agent || $sel_agent || $FS::CurrentUser::CurrentUser->a
my $row_agentnum = $all_agent || $agent->agentnum;
my $row_refnum = $all_part_referral || $part_referral->refnum;
- push @params, [
+ my @row_params = (
@base_params,
$class_param => $row_classnum,
($all_agent ? () : ('agentnum' => $row_agentnum) ),
($all_part_referral ? () : ('refnum' => $row_refnum)),
'charges' => $component,
- ];
-
- push @links, "$link;".
+ );
+ my $row_link = "$link;".
($all_agent ? '' : "agentnum=$row_agentnum;").
($all_part_referral ? '' : "refnum=$row_refnum;").
(join('',map {"cust_classnum=$_;"} @cust_classnums)).
"$class_param=$row_classnum;".
"distribute=$distribute;".
"use_override=$use_override;charges=$component;";
+ if ( $class_param eq 'report_optionnum' ) {
+ push @row_params,
+ 'all_report_options' => 1,
+ 'not_report_optionnum' => $not_row_classnum,
+ ;
+ $row_link .= "all_report_options=1;".
+ "not_report_optionnum=$not_row_classnum;";
+ }
+ push @params, \@row_params;
+ push @links, $row_link;
@colorbuf = @agent_colors unless @colorbuf;
push @colors, shift @colorbuf;
diff --git a/httemplate/graph/report_cust_bill_pkg.html b/httemplate/graph/report_cust_bill_pkg.html
index 1e54df3..c6eb0f2 100644
--- a/httemplate/graph/report_cust_bill_pkg.html
+++ b/httemplate/graph/report_cust_bill_pkg.html
@@ -64,12 +64,15 @@ function class_mode_changed() {
var div_pkg = document.getElementById('pkg_class');
var div_report = document.getElementById('report_class');
+ var span_exact = document.getElementById('exact_match');
if (mode == 'pkg') {
div_pkg.style.display = '';
div_report.style.display = 'none';
+ span_exact.style.display = 'none';
} else if (mode == 'report') {
div_pkg.style.display = 'none';
div_report.style.display = '';
+ span_exact.style.display = '';
}
}
window.onload = class_mode_changed;
@@ -149,6 +152,11 @@ window.onload = class_mode_changed;
<BR>
<INPUT TYPE="radio" NAME="class_agg_break" ID="class_agg_break_breakdown" VALUE="breakdown" onchange="enable_agent_totals(this)">
<% emt('Breakdown') %>
+ <BR>
+ <SPAN ID="exact_match" style="display:none">
+ <INPUT TYPE="radio" NAME="class_agg_break" ID="class_agg_break_exact" VALUE="exact" onchange="enable_agent_totals(this)">
+ <% emt('Exact match') %>
+ </SPAN>
</TD>
</TR>
diff --git a/httemplate/search/cust_bill_pkg.cgi b/httemplate/search/cust_bill_pkg.cgi
index f87862a..61093d2 100644
--- a/httemplate/search/cust_bill_pkg.cgi
+++ b/httemplate/search/cust_bill_pkg.cgi
@@ -311,7 +311,9 @@ my $join_pkg =
LEFT JOIN part_pkg USING (pkgpart)';
my $part_pkg = 'part_pkg';
-if ( $cgi->param('use_override') ) { #"Separate sub-packages from parents"
+# "Separate sub-packages from parents"
+my $use_override = $cgi->param('use_override') ? 1 : 0;
+if ( $use_override ) {
# still need the real part_pkg for tax applicability,
# so alias this one
$join_pkg .= " LEFT JOIN part_pkg AS override ON (
@@ -342,14 +344,16 @@ if ( $cgi->param('nottax') ) {
}
if ( grep { $_ eq 'report_optionnum' } $cgi->param ) {
- my @nums = grep /^\w+$/, $cgi->param('report_optionnum');
- my $num = join(',', @nums);
+ my $num = join(',', grep /^[\d,]+$/, $cgi->param('report_optionnum'));
+ my $not_num = join(',', grep /^[\d,]+$/, $cgi->param('not_report_optionnum'));
+ my $all = $cgi->param('all_report_options') ? 1 : 0;
push @where, # code reuse FTW
- FS::Report::Table->with_report_option( $num, $cgi->param('use_override'));
- }
-
- if ( $cgi->param('report_optionnum') =~ /^(\w+)$/ ) {
- ;
+ FS::Report::Table->with_report_option(
+ report_optionnum => $num,
+ not_report_optionnum => $not_num,
+ use_override => $use_override,
+ all_report_options => $all,
+ );
}
# taxclass
-----------------------------------------------------------------------
Summary of changes:
FS/FS/Report/Table.pm | 105 +++++++++++++++++----------
httemplate/graph/cust_bill_pkg.cgi | 83 +++++++++++++++++-----
httemplate/graph/report_cust_bill_pkg.html | 8 ++
httemplate/search/cust_bill_pkg.cgi | 20 +++--
4 files changed, 149 insertions(+), 67 deletions(-)
More information about the freeside-commits
mailing list