From: Mark Wells Date: Fri, 14 Aug 2015 21:43:02 +0000 (-0700) Subject: fix anomalous behavior in line item report with consolidated taxes, needed for #26770... X-Git-Url: http://git.freeside.biz/gitweb/?p=freeside.git;a=commitdiff_plain;h=039381569561964c572407409fe61f6d4da97afe fix anomalous behavior in line item report with consolidated taxes, needed for #26770, from...#18676, I think? --- diff --git a/httemplate/search/cust_bill_pkg.cgi b/httemplate/search/cust_bill_pkg.cgi index c59a6d0e7..ac686eab8 100644 --- a/httemplate/search/cust_bill_pkg.cgi +++ b/httemplate/search/cust_bill_pkg.cgi @@ -461,6 +461,9 @@ if ( $cgi->param('nottax') ) { } + # This is the only place we should attempt to show credits on here: + # the total of credit applications to the line item. + my $credit_sub = 'SELECT SUM(amount) AS credit_amount, billpkgnum FROM cust_credit_bill_pkg GROUP BY billpkgnum'; @@ -579,51 +582,51 @@ if ( $cgi->param('nottax') ) { } elsif ( $cgi->param('istax') ) { - @peritem = ( 'setup' ); # taxes only have setup - @peritem_desc = ( 'Tax charge' ); + # ensure that it is a tax: + push @where, 'cust_bill_pkg.pkgnum = 0', + 'cust_bill_pkg.feepart IS NULL'; + + # We MUST NOT join cust_bill_pkg to any table that it's 1:many to. + # Otherwise we get duplication of the cust_bill_pkg records, + # inaccurate totals, nonsensical paging behavior, etc. + # We CAN safely join it to a subquery that has unique billpkgnums, and + # that's what we'll do. - push @where, 'cust_bill_pkg.pkgnum = 0'; + my $tax_subquery; + my @tax_where; # tax location when using tax_rate_location if ( $cgi->param('vendortax') ) { - $join_pkg .= ' LEFT JOIN cust_bill_pkg_tax_rate_location USING ( billpkgnum ) '. - ' LEFT JOIN tax_rate_location USING ( taxratelocationnum )'; + $tax_subquery = ' + SELECT billpkgnum, SUM(amount) as tax_total + FROM cust_bill_pkg_tax_rate_location AS tax + JOIN tax_rate_location USING (taxratelocationnum) + '; foreach (qw( state county city locationtaxid)) { if ( scalar($cgi->param($_)) ) { my $place = dbh->quote( $cgi->param($_) ); - push @where, "tax_rate_location.$_ = $place"; + push @tax_where, "tax_rate_location.$_ = $place"; } } - push @total, 'SUM( - COALESCE(cust_bill_pkg_tax_rate_location.amount, - cust_bill_pkg.setup + cust_bill_pkg.recur) - )'; - push @total_desc, "$money_char%.2f total"; - } else { # the internal-tax case - $join_pkg .= ' - LEFT JOIN cust_bill_pkg_tax_location USING (billpkgnum) - JOIN cust_main_county USING (taxnum) - '; - - # don't double-count the components of consolidated taxes - @total = ( 'COUNT(DISTINCT cust_bill_pkg.billpkgnum)', - 'SUM(cust_bill_pkg_tax_location.amount)' ); - @total_desc = "$money_char%.2f total"; + my $tax_select = 'SELECT tax.billpkgnum, SUM(tax.amount) as tax_total'; + my $tax_from = ' FROM cust_bill_pkg_tax_location AS tax JOIN cust_main_county USING (taxnum)'; # package classnum if ( grep { $_ eq 'classnum' } $cgi->param ) { my @classnums = grep /^\d*$/, $cgi->param('classnum'); - $join_pkg .= ' - JOIN cust_pkg AS taxed_pkg - ON (cust_bill_pkg_tax_location.pkgnum = taxed_pkg.pkgnum) - JOIN part_pkg AS taxed_part_pkg - ON (taxed_pkg.pkgpart = taxed_part_pkg.pkgpart) + $tax_from .= ' + JOIN cust_bill_pkg AS taxed_item + ON (tax.taxable_billpkgnum = taxed_item.billpkgnum) + LEFT JOIN cust_pkg AS taxed_pkg ON (taxed_item.pkgnum = taxed_pkg.pkgnum) + LEFT JOIN part_pkg AS taxed_part_pkg ON (taxed_pkg.pkgpart = taxed_part_pkg.pkgpart) + LEFT JOIN part_fee AS taxed_part_fee ON (taxed_item.feepart = taxed_part_fee.feepart) '; - push @where, "COALESCE(taxed_part_pkg.classnum, 0) IN ( ". + push @tax_where, + "COALESCE(taxed_part_pkg.classnum, taxed_part_fee.classnum,0) IN ( ". join(',', @classnums ). ' )' if @classnums; @@ -631,19 +634,20 @@ if ( $cgi->param('nottax') ) { # taxclass if ( $cgi->param('taxclassNULL') ) { - push @where, 'cust_main_county.taxclass IS NULL'; + push @tax_where, 'cust_main_county.taxclass IS NULL'; } # taxname if ( $cgi->param('taxnameNULL') ) { - push @where, 'cust_main_county.taxname IS NULL OR '. + push @tax_where, 'cust_main_county.taxname IS NULL OR '. 'cust_main_county.taxname = \'Tax\''; } elsif ( $cgi->param('taxname') ) { - push @where, 'cust_main_county.taxname = '. + push @tax_where, 'cust_main_county.taxname = '. dbh->quote($cgi->param('taxname')); } # itemdesc, for breakdown from the vendor tax report + # (is this even used? vendor tax report shouldn't use cust_bill_pkg.cgi) if ( $cgi->param('itemdesc') ) { if ( $cgi->param('itemdesc') eq 'Tax' ) { push @where, "($itemdesc = 'Tax' OR $itemdesc is null)"; @@ -652,103 +656,54 @@ if ( $cgi->param('nottax') ) { } } - # specific taxnums + # specific taxnums (the usual way) if ( $cgi->param('taxnum') =~ /^([\d,]+)$/) { - push @where, "cust_main_county.taxnum IN ($1)"; + push @tax_where, "cust_main_county.taxnum IN ($1)"; } - } #end of "normal case" - -} # nottax / istax - - -#total payments -my $pay_sub = "SELECT SUM(cust_bill_pay_pkg.amount) - FROM cust_bill_pay_pkg - WHERE cust_bill_pkg.billpkgnum = cust_bill_pay_pkg.billpkgnum - "; -push @select, "($pay_sub) AS pay_amount"; - - -# credit -if ( $cgi->param('credit') ) { + $tax_subquery = "$tax_select $tax_from"; - my $credit_where; + } # end of internal-tax case - my($cr_begin, $cr_end) = FS::UI::Web::parse_beginning_ending($cgi, 'credit'); - $credit_where = "WHERE cust_credit_bill._date >= $cr_begin " . - "AND cust_credit_bill._date <= $cr_end"; - - my $credit_sub; - - if ( $cgi->param('istax') ) { - # then we need to group/join by billpkgtaxlocationnum, to get only the - # relevant part of partial taxes - my $credit_sub = "SELECT SUM(cust_credit_bill_pkg.amount) AS credit_amount, - reason.reason as reason_text, access_user.username AS username_text, - billpkgtaxlocationnum, billpkgnum - FROM cust_credit_bill_pkg - JOIN cust_credit_bill USING (creditbillnum) - JOIN cust_credit USING (crednum) - LEFT JOIN reason USING (reasonnum) - LEFT JOIN access_user USING (usernum) - $credit_where - GROUP BY billpkgnum, billpkgtaxlocationnum, reason.reason, - access_user.username"; - - if ( $cgi->param('out') ) { + if (@tax_where) { + $tax_subquery .= ' + WHERE ' . join(' AND ', map "($_)", @tax_where); + } + $tax_subquery .= ' GROUP BY tax.billpkgnum '; - # find credits that are applied to the line items, but not to - # a cust_bill_pkg_tax_location link - $join_pkg .= " LEFT JOIN ($credit_sub) AS item_credit - USING (billpkgnum)"; - push @where, 'item_credit.billpkgtaxlocationnum IS NULL'; + # now join THAT into the main report + # (inner join, so that tax line items that don't match the tax_where + # conditions don't appear in the output.) - } else { + $join_pkg .= " JOIN ($tax_subquery) AS _istax USING (billpkgnum) "; - # find credits that are applied to the CBPTL links that are - # considered "interesting" by the report criteria - $join_pkg .= " LEFT JOIN ($credit_sub) AS item_credit - USING (billpkgtaxlocationnum)"; + push @select, 'tax_total'; - } + @peritem = ( 'setup' ); # total tax on the invoice, not just the filtered tax + @peritem_desc = ( 'Tax charge' ); - } else { - # then only group by billpkgnum - my $credit_sub = "SELECT SUM(cust_credit_bill_pkg.amount) AS credit_amount, - reason.reason as reason_text, access_user.username AS username_text, - billpkgnum - FROM cust_credit_bill_pkg - JOIN cust_credit_bill USING (creditbillnum) - JOIN cust_credit USING (crednum) - LEFT JOIN reason USING (reasonnum) - LEFT JOIN access_user USING (usernum) - $credit_where - GROUP BY billpkgnum, reason.reason, access_user.username"; - $join_pkg .= " LEFT JOIN ($credit_sub) AS item_credit USING (billpkgnum)"; + @total = ( 'COUNT(cust_bill_pkg.billpkgnum)', + 'SUM(cust_bill_pkg.setup)' ); + @total_desc = ( "$money_char%.2f total tax" ); + + if ( @tax_where ) { + # then also show the filtered tax + push @peritem, 'tax_total'; + push @peritem_desc, 'Tax in category'; + push @total, 'SUM(tax_total)'; + push @total_desc, "$money_char%.2f tax in this category"; + # would also be nice to include a line explaining what the category is } - push @where, 'item_credit.billpkgnum IS NOT NULL'; - push @select, 'item_credit.credit_amount', - 'item_credit.username_text', - 'item_credit.reason_text'; - push @peritem, 'credit_amount', 'username_text', 'reason_text'; - push @peritem_desc, 'Credited', 'By', 'Reason'; - push @total, 'SUM(credit_amount)'; - push @total_desc, "$money_char%.2f credited"; - -} else { - - #still want a credit total column +} # nottax / istax - my $credit_sub = " - SELECT SUM(cust_credit_bill_pkg.amount) - FROM cust_credit_bill_pkg - WHERE cust_bill_pkg.billpkgnum = cust_credit_bill_pkg.billpkgnum - "; - push @select, "($credit_sub) AS credit_amount"; +#total payments +my $pay_sub = "SELECT SUM(cust_bill_pay_pkg.amount) + FROM cust_bill_pay_pkg + WHERE cust_bill_pkg.billpkgnum = cust_bill_pay_pkg.billpkgnum + "; +push @select, "($pay_sub) AS pay_amount"; -} push @select, 'cust_main.custnum', FS::UI::Web::cust_sql_fields(); diff --git a/httemplate/search/report_tax-xls.cgi b/httemplate/search/report_tax-xls.cgi index d0ef434f4..743f14788 100755 --- a/httemplate/search/report_tax-xls.cgi +++ b/httemplate/search/report_tax-xls.cgi @@ -146,7 +146,7 @@ my $colhead = $format[0]->{colhead}; # print header $ws->merge_range($y, 1, $y, 5, 'Sales', $colhead); $ws->merge_range($y, 6, $y+1, 8, 'Rate', $colhead); -$ws->merge_range($y, 9, $y, 14, 'Tax', $colhead); +$ws->merge_range($y, 9, $y, 15, 'Tax', $colhead); $y++; $colhead = $format[0]->{colhead_small}; @@ -156,16 +156,17 @@ $ws->write($y, 9, 'Estimated', $colhead); $ws->write($y, 10, 'Invoiced', $colhead); $ws->write($y, 12, 'Credited', $colhead); $ws->write($y, 14, 'Net due', $colhead); +$ws->write($y, 15, 'Collected',$colhead); $y++; # print data -my $rownum = 0; +my $rownum = 1; my $prev_row = { pkgclass => 'DUMMY PKGCLASS' }; foreach my $row (@rows) { $x = 0; if ( $row->{pkgclass} ne $prev_row->{pkgclass} ) { - $rownum = 0; + $rownum = 1; if ( $params{breakdown}->{pkgclass} ) { $ws->merge_range($y, 0, $y, 14, $pkgclass_name{$row->{pkgclass}}, @@ -206,6 +207,8 @@ foreach my $row (@rows) { $ws->write_string($y, $x, " = ", $f->{bigmath}); $x++; $ws->write($y, $x, $row->{tax} - $row->{credit}, $f->{currency}); + $x++; + $ws->write($y, $x, $row->{tax_paid} || 0, $f->{currency}); $rownum++; $y++; diff --git a/httemplate/search/report_tax.cgi b/httemplate/search/report_tax.cgi index 491cd42c5..1d906473e 100644 --- a/httemplate/search/report_tax.cgi +++ b/httemplate/search/report_tax.cgi @@ -32,6 +32,8 @@ TD.rowhead { font-weight: bold; text-align: left; padding: 0px 3px } Tax credited Net tax due + + Tax collected @@ -131,13 +133,16 @@ TD.rowhead { font-weight: bold; text-align: left; padding: 0px 3px } % # credited tax − - +%# currently broken <% $money_sprintf->( $row->{credit} ) %> - +%# % # net tax due = <% $money_sprintf->( $row->{tax} - $row->{credit} ) %> +% # tax collected +   + <% $money_sprintf->( $row->{tax_paid} ) %> % $rownum++; % $prev_row = $row; @@ -218,12 +223,12 @@ if ( $params{agentnum} ) { my $saleslink = $p. "search/cust_bill_pkg.cgi?$dateagentlink;nottax=1"; my $taxlink = $p. "search/cust_bill_pkg.cgi?$dateagentlink;istax=1"; my $exemptlink = $p. "search/cust_tax_exempt_pkg.cgi?$dateagentlink"; -my $creditlink = $p. "search/cust_bill_pkg.cgi?$dateagentlink;credit=1;istax=1"; - -if ( $params{'credit_date'} eq 'cust_credit_bill' ) { - $creditlink =~ s/begin/credit_begin/; - $creditlink =~ s/end/credit_end/; -} +#my $creditlink = $p. "search/cust_bill_pkg.cgi?$dateagentlink;credit=1;istax=1"; +#if ( $params{'credit_date'} eq 'cust_credit_bill' ) { +# $creditlink =~ s/begin/credit_begin/; +# $creditlink =~ s/end/credit_end/; +#} +my $creditlink = ''; # disabled until we find a sane way to do this my %pkgclass_name = map { $_->classnum, $_->classname } qsearch('pkg_class'); $pkgclass_name{''} = 'Unclassified';