[freeside-commits] branch FREESIDE_3_BRANCH updated. dd0bf7c0db2d97a33bedd324788ca550b54572fc
Mark Wells
mark at 420.am
Thu Jan 9 16:38:00 PST 2014
The branch, FREESIDE_3_BRANCH has been updated
via dd0bf7c0db2d97a33bedd324788ca550b54572fc (commit)
from 3f5067134c948ce709324d88421d54217e14ca21 (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 dd0bf7c0db2d97a33bedd324788ca550b54572fc
Author: Mark Wells <mark at freeside.biz>
Date: Thu Jan 9 16:37:13 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 6e110d6..ab23e52 100644
--- a/httemplate/search/cust_bill_pkg.cgi
+++ b/httemplate/search/cust_bill_pkg.cgi
@@ -287,7 +287,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 (
@@ -318,14 +320,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