From 4ad29235ceb48ec0c5a0af07e6ccfcb64b40f466 Mon Sep 17 00:00:00 2001 From: jeff Date: Sat, 30 Aug 2008 21:34:45 +0000 Subject: [PATCH] remove duplicate cust_bill_pkg creation RT#3919 --- FS/FS/Conf.pm | 7 +++ FS/FS/Report/Table/Monthly.pm | 1 - FS/FS/cust_bill.pm | 72 +++++++++++++++++++------ FS/FS/cust_bill_pkg.pm | 10 ++-- FS/FS/cust_main.pm | 81 ++++++++--------------------- FS/FS/part_pkg/voip_cdr.pm | 68 ++---------------------- httemplate/search/cust_bill_pkg.cgi | 2 - httemplate/search/report_prepaid_income.cgi | 5 +- httemplate/search/report_tax.cgi | 3 +- 9 files changed, 98 insertions(+), 151 deletions(-) diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm index 08b299fba..11286a89a 100644 --- a/FS/FS/Conf.pm +++ b/FS/FS/Conf.pm @@ -866,6 +866,13 @@ worry that config_items is freeside-specific and icky. 'type' => 'checkbox', }, + { + 'key' => 'separate_usage', + 'section' => 'billing', + 'description' => 'Split the rated call usage into a separate line from the recurring charges.', + 'type' => 'checkbox', + }, + { 'key' => 'payment_receipt_email', 'section' => 'billing', diff --git a/FS/FS/Report/Table/Monthly.pm b/FS/FS/Report/Table/Monthly.pm index 0b8fca975..d75f0be79 100644 --- a/FS/FS/Report/Table/Monthly.pm +++ b/FS/FS/Report/Table/Monthly.pm @@ -329,7 +329,6 @@ sub cust_bill_pkg { LEFT JOIN cust_pkg USING ( pkgnum ) LEFT JOIN part_pkg USING ( pkgpart ) WHERE pkgnum != 0 - AND ( duplicate IS NULL OR duplicate = '' ) AND $where AND ". $self->in_time_period_and_agent($speriod, $eperiod, $agentnum) ); diff --git a/FS/FS/cust_bill.pm b/FS/FS/cust_bill.pm index 3274d3875..6b2bcd675 100644 --- a/FS/FS/cust_bill.pm +++ b/FS/FS/cust_bill.pm @@ -2555,6 +2555,15 @@ sub _items_sections { if ( $cust_bill_pkg->pkgnum > 0 ) { my $desc = $cust_bill_pkg->section; + my $dup_desc = $cust_bill_pkg->duplicate_section; + + if ($cust_bill_pkg->duplicate) { + $s{$dup_desc} += $cust_bill_pkg->setup + if ( $cust_bill_pkg->setup != 0 ); + + $s{$dup_desc} += $cust_bill_pkg->recur + if ( $cust_bill_pkg->recur != 0 ); + } if ( $cust_bill_pkg->post_total ) { $l{$desc} += $cust_bill_pkg->setup @@ -2634,11 +2643,12 @@ sub _items_previous { sub _items_pkg { my $self = shift; my %options = @_; - my $section = delete $options{'section'}; + my $section = $options{'section'}; + my $desc = $section->{'description'}; my @cust_bill_pkg = grep { $_->pkgnum && ( defined($section) - ? $_->section eq $section->{'description'} + ? ( $_->section eq $desc || $_->duplicate_section eq $desc ) : 1 ) } $self->cust_bill_pkg; @@ -2671,6 +2681,7 @@ sub _items_cust_bill_pkg { my $unsquelched = $opt{unsquelched} || ''; my @b = (); + my $last_pkgnum = ''; foreach my $cust_bill_pkg ( grep { $unsquelched ? 1 : ! $_->separate_cdr } @$cust_bill_pkg ) @@ -2706,11 +2717,19 @@ sub _items_cust_bill_pkg { quantity => $cust_bill_pkg->quantity, ext_description => \@d, }; + + $last_pkgnum = ''; + } if ( $cust_bill_pkg->recur != 0 ) { - my $description = $desc; + my $is_summary = + ( $cust_bill_pkg->duplicate && + $opt{section}->{description} ne $cust_bill_pkg->section + ); + my $description = $is_summary ? "Usage charges" : $desc; + unless ( $conf->exists('disable_line_item_date_ranges') ) { $description .= " (" . time2str("%x", $cust_bill_pkg->sdate). " - ". time2str("%x", $cust_bill_pkg->edate). ")"; @@ -2718,23 +2737,42 @@ sub _items_cust_bill_pkg { #at least until cust_bill_pkg has "past" ranges in addition to #the "future" sdate/edate ones... see #3032 - my @d = map &{$escape_function}($_), - $cust_pkg->h_labels_short($self->_date); + my @d = (); + push @d, map &{$escape_function}($_), + $cust_pkg->h_labels_short($self->_date) #$cust_bill_pkg->edate, #$cust_bill_pkg->sdate), - @d = () if $cust_bill_pkg->itemdesc; - push @d, $cust_bill_pkg->details(%details_opt); + unless ($cust_bill_pkg->pkgnum eq $last_pkgnum); - push @b, { - description => $description, - #pkgpart => $part_pkg->pkgpart, - pkgnum => $cust_bill_pkg->pkgnum, - amount => sprintf("%.2f", $cust_bill_pkg->recur), - unit_amount => sprintf("%.2f", $cust_bill_pkg->unitrecur), - quantity => $cust_bill_pkg->quantity, - ext_description => \@d, - }; + @d = () if ($cust_bill_pkg->itemdesc || $is_summary); + push @d, $cust_bill_pkg->details(%details_opt) + unless $is_summary; + + if ($cust_bill_pkg->pkgnum eq $last_pkgnum) { + + $b[$#b]->{amount} = + sprintf("%.2f", $b[$#b]->{amount} + $cust_bill_pkg->recur); + push @{$b[$#b]->{ext_description}}, @d; + }else{ + + push @b, { + description => $description, + #pkgpart => $part_pkg->pkgpart, + pkgnum => $cust_bill_pkg->pkgnum, + amount => sprintf("%.2f", $cust_bill_pkg->recur), + unit_amount => sprintf("%.2f", $cust_bill_pkg->unitrecur), + quantity => $cust_bill_pkg->quantity, + ext_description => \@d, + }; + + } + + if ($conf->exists('separate_usage') && $cust_bill_pkg->type ne 'U') { + $last_pkgnum = ''; + }else{ + $last_pkgnum = $cust_bill_pkg->pkgnum; + } } } else { #pkgnum tax or one-shot line item (??) @@ -2754,6 +2792,8 @@ sub _items_cust_bill_pkg { }; } + $last_pkgnum = ''; + } } diff --git a/FS/FS/cust_bill_pkg.pm b/FS/FS/cust_bill_pkg.pm index 6925de8b3..aa37ac4b5 100644 --- a/FS/FS/cust_bill_pkg.pm +++ b/FS/FS/cust_bill_pkg.pm @@ -61,8 +61,7 @@ supported: =item section - Invoice section (overrides normal package section) -=duplicate - Indicates this item appears elsewhere on the invoice - (and should not be retaxed or reincluded in totals) +=duplicate - Indicates this item is a candidate for summarizing and duplicating at print time =post_total - A hint that this item should appear after invoice totals @@ -77,6 +76,11 @@ sub section { } } +sub duplicate_section { + my $self = shift; + $self->duplicate ? $self->part_pkg->categoryname : ''; +} + =item quantity - If not set, defaults to 1 =item unitsetup - If not set, defaults to setup @@ -375,7 +379,7 @@ sub owed_recur { # modeled after cust_bill::owed... sub owed { my( $self, $field ) = @_; - my $balance = $self->duplicate ? 0 : $self->$field(); + my $balance = $self->$field(); $balance -= $_->amount foreach ( $self->cust_bill_pay_pkg($field) ); $balance -= $_->amount foreach ( $self->cust_credit_bill_pkg($field) ); $balance = sprintf( '%.2f', $balance ); diff --git a/FS/FS/cust_main.pm b/FS/FS/cust_main.pm index 718fccf2f..e9e21b80b 100644 --- a/FS/FS/cust_main.pm +++ b/FS/FS/cust_main.pm @@ -2068,7 +2068,6 @@ sub bill { $self->select_for_update; #mutex my @cust_bill_pkg = (); - my @appended_cust_bill_pkg = (); ### # find the packages which are due for billing, find out how much they are @@ -2107,7 +2106,6 @@ sub bill { 'cust_pkg' => $cust_pkg, 'precommit_hooks' => \@precommit_hooks, 'line_items' => \@cust_bill_pkg, - 'appended_line_items' => \@appended_cust_bill_pkg, 'setup' => \$total_setup, 'recur' => \$total_recur, 'tax_matrix' => \%taxlisthash, @@ -2123,8 +2121,6 @@ sub bill { } #foreach my $cust_pkg - push @cust_bill_pkg, @appended_cust_bill_pkg; - unless ( @cust_bill_pkg ) { #don't create an invoice w/o line items #but do commit any package date cycling that happened $dbh->commit or die $dbh->errstr if $oldAutoCommit; @@ -2144,7 +2140,6 @@ sub bill { 'cust_pkg' => $postal_pkg, 'precommit_hooks' => \@precommit_hooks, 'line_items' => \@cust_bill_pkg, - 'appended_line_items' => \@appended_cust_bill_pkg, 'setup' => \$total_setup, 'recur' => \$total_recur, 'tax_matrix' => \%taxlisthash, @@ -2296,8 +2291,6 @@ sub _make_lines { my $cust_pkg = $params{cust_pkg} or die "no cust_pkg specified"; my $precommit_hooks = $params{precommit_hooks} or die "no package specified"; my $cust_bill_pkgs = $params{line_items} or die "no line buffer specified"; - my $appended_cust_bill_pkg = $params{appended_line_items} - or die "no appended line buffer specified"; my $total_setup = $params{setup} or die "no setup accumulator specified"; my $total_recur = $params{recur} or die "no recur accumulator specified"; my $taxlisthash = $params{tax_matrix} or die "no tax accumulator specified"; @@ -2448,6 +2441,7 @@ sub _make_lines { warn " charges (setup=$setup, recur=$recur); adding line items\n" if $DEBUG > 1; + my $cust_bill_pkg = new FS::cust_bill_pkg { 'pkgnum' => $cust_pkg->pkgnum, 'setup' => $setup, @@ -2469,61 +2463,19 @@ sub _make_lines { # handle taxes ### - unless ( $self->tax =~ /Y/i || $self->payby eq 'COMP' ) { - - #some garbage disappears on cust_bill_pkg refactor - my $err_or_cust_bill_pkg = - $self->_handle_taxes($part_pkg, $taxlisthash, $cust_bill_pkg); - - return $err_or_cust_bill_pkg - unless ( ref($err_or_cust_bill_pkg) ); + my $err_or_cust_bill_pkg = + $self->_handle_taxes($part_pkg, $taxlisthash, $cust_bill_pkg, $cust_pkg); - push @$cust_bill_pkgs, @$err_or_cust_bill_pkg; + return $err_or_cust_bill_pkg + unless ( ref($err_or_cust_bill_pkg) ); - } #unless $self->tax =~ /Y/i || $self->payby eq 'COMP' + push @$cust_bill_pkgs, @$err_or_cust_bill_pkg; } #if $setup != 0 || $recur != 0 } #if $line_items - if ( $part_pkg->can('append_cust_bill_pkgs') ) { - my %param = ( 'precommit_hooks' => $precommit_hooks, ); - my ($more_cust_bill_pkgs) = - eval { $part_pkg->append_cust_bill_pkgs( $cust_pkg, \$sdate, \%param ) }; - - return "$@ running append_cust_bill_pkgs for $cust_pkg\n" - if ( $@ ); - return "$more_cust_bill_pkgs" - unless ( ref($more_cust_bill_pkgs) ); - - foreach my $cust_bill_pkg ( @{$more_cust_bill_pkgs} ) { - - $cust_bill_pkg->pkgpart_override($part_pkg->pkgpart) - unless $part_pkg->pkgpart == $real_pkgpart; - - unless ($cust_bill_pkg->duplicate) { - $$total_setup += $cust_bill_pkg->setup; - $$total_recur += $cust_bill_pkg->recur; - - ### - # handle taxes - ### - - unless ( $self->tax =~ /Y/i || $self->payby eq 'COMP' ) { - - #some garbage disappears on cust_bill_pkg refactor - my $err_or_cust_bill_pkg = - $self->_handle_taxes($part_pkg, $taxlisthash, $cust_bill_pkg); - - return $err_or_cust_bill_pkg - unless ( ref($err_or_cust_bill_pkg) ); - - push @$appended_cust_bill_pkg, @$err_or_cust_bill_pkg; - - } #unless $self->tax =~ /Y/i || $self->payby eq 'COMP' - } - } - } + ''; } @@ -2532,6 +2484,7 @@ sub _handle_taxes { my $part_pkg = shift; my $taxlisthash = shift; my $cust_bill_pkg = shift; + my $cust_pkg = shift; my %cust_bill_pkg = (); my %taxes = (); @@ -2549,6 +2502,7 @@ sub _handle_taxes { if ( $conf->exists('enable_taxproducts') && (scalar($part_pkg->part_pkg_taxoverride) || $part_pkg->has_taxproduct) + && ( $self->tax !~ /Y/i && $self->payby ne 'COMP' ) ) { @@ -2564,7 +2518,7 @@ sub _handle_taxes { $taxes{''} = $err_or_ref; } - }else{ + }elsif ( $self->tax !~ /Y/i && $self->payby ne 'COMP' ) { my %taxhash = map { $_ => $self->get("$prefix$_") } qw( state county country ); @@ -2599,10 +2553,16 @@ sub _handle_taxes { $part_pkg->taxclass ). "\n"; } - } #if $conf->exists('enable_taxproducts') - - # XXX all this goes away with cust_bill_pay refactor + } #if $conf->exists('enable_taxproducts') ... + + my $section = $cust_pkg->part_pkg->option('usage_section', 'Hush!') + if $cust_pkg->part_pkg->option('separate_usage'); + my $want_duplicate = + $cust_pkg->part_pkg->option('summarize_usage', 'Hush!') && + $cust_pkg->part_pkg->option('usage_section', 'Hush!'); + # XXX this mostly goes away with cust_bill_pkg refactor + $cust_bill_pkg{setup} = $cust_bill_pkg if $cust_bill_pkg->setup; $cust_bill_pkg{recur} = $cust_bill_pkg if $cust_bill_pkg->recur; @@ -2626,6 +2586,9 @@ sub _handle_taxes { new FS::cust_bill_pkg { $cust_bill_pkg{recur}->hash }; $cust_bill_pkg_usage->recur( $usage ); $cust_bill_pkg_usage->type( 'U' ); + $cust_bill_pkg_usage->duplicate( $want_duplicate ? 'Y' : '' ); + $cust_bill_pkg_usage->section( $section ); + $cust_bill_pkg_usage->post_total( $want_duplicate ? 'Y' : '' ); my $recur = sprintf( "%.2f", $cust_bill_pkg{recur}->recur - $usage ); $cust_bill_pkg{recur}->recur( $recur ); $cust_bill_pkg{recur}->type( '' ); diff --git a/FS/FS/part_pkg/voip_cdr.pm b/FS/FS/part_pkg/voip_cdr.pm index 10257056d..bd73ea604 100644 --- a/FS/FS/part_pkg/voip_cdr.pm +++ b/FS/FS/part_pkg/voip_cdr.pm @@ -85,10 +85,6 @@ tie my %rating_method, 'Tie::IxHash', 'select_options' => { FS::cdr::invoice_formats() }, }, - 'separate_usage' => { 'name' => 'Separate usage charges from recurring charges', - 'type' => 'checkbox', - }, - 'usage_section' => { 'name' => 'Section in which to place separate usage charges', }, @@ -127,7 +123,7 @@ tie my %rating_method, 'Tie::IxHash', disable_src domestic_prefix international_prefix use_amaflags use_disposition output_format - separate_usage summarize_usage usage_section + summarize_usage usage_section ) ], 'weight' => 40, @@ -138,16 +134,8 @@ sub calc_setup { $self->option('setup_fee'); } -sub calc_recur { - my $self = shift; - my $charges = 0; - $charges = $self->calc_usage(@_) - unless $self->option('separate_usage', 'Hush!'); - $self->option('recur_fee') + $charges; -} - #false laziness w/voip_sqlradacct calc_recur resolve it if that one ever gets used again -sub calc_usage { +sub calc_recur { my($self, $cust_pkg, $sdate, $details, $param ) = @_; my $last_bill = $cust_pkg->last_bill; @@ -452,7 +440,7 @@ sub calc_usage { } #if ( $spool_cdr && length($downstream_cdr) ) - $charges; + $self->option('recur_fee') + $charges; } @@ -472,55 +460,5 @@ sub calc_units { scalar(grep { $_->part_svc->svcdb eq 'svc_phone' } $cust_pkg->cust_svc); } -sub append_cust_bill_pkgs { - my $self = shift; - my($cust_pkg, $sdate, $details, $param ) = @_; - return [] - unless $self->option('separate_usage', 'Hush!'); - - my @details = (); - my $charges = $self->calc_usage($cust_pkg, $sdate, \@details, $param); - - return [] - unless $charges; # unless @details? - - my @cust_bill_pkg = (); - - my $want_summary = $self->option('summarize_usage', 'Hush!') && - $self->option('usage_section', 'Hush!'); - - push @cust_bill_pkg, new FS::cust_bill_pkg { - 'pkgnum' => $cust_pkg->pkgnum, - 'setup' => 0, - 'unitsetup' => 0, - 'recur' => sprintf( "%.2f", $charges), # hmmm - 'unitrecur' => 0, - 'quantity' => $cust_pkg->quantity, - 'sdate' => $$sdate, - 'edate' => $cust_pkg->bill, # already fiddled - 'itemdesc' => 'Usage charges', # configurable? - 'duplicate' => 'Y', - } - if $want_summary; - - push @cust_bill_pkg, new FS::cust_bill_pkg { - 'pkgnum' => $cust_pkg->pkgnum, - 'setup' => 0, - 'unitsetup ' => 0, - 'recur' => sprintf( "%.2f", $charges), # hmmm - 'unitrecur ' => 0, - 'quantity' => $cust_pkg->quantity, - 'sdate' => $$sdate, - 'edate' => $cust_pkg->bill, # already fiddled - 'itemdesc' => 'Usage charges', # configurable? - 'section' => $self->option('usage_section', 'Hush!'), - 'details' => \@details, - 'post_total' => ( $want_summary ? 'Y' : '' ), - }; - - - return [ @cust_bill_pkg ]; -} - 1; diff --git a/httemplate/search/cust_bill_pkg.cgi b/httemplate/search/cust_bill_pkg.cgi index 2354be953..74efe4f7e 100644 --- a/httemplate/search/cust_bill_pkg.cgi +++ b/httemplate/search/cust_bill_pkg.cgi @@ -70,8 +70,6 @@ my $agentnums_sql = my @where = ( $agentnums_sql ); -push @where, "(duplicate IS NULL OR duplicate = '' )"; - my($beginning, $ending) = FS::UI::Web::parse_beginning_ending($cgi); push @where, "_date >= $beginning", "_date <= $ending"; diff --git a/httemplate/search/report_prepaid_income.cgi b/httemplate/search/report_prepaid_income.cgi index 3694c7aae..27dbcbf9f 100644 --- a/httemplate/search/report_prepaid_income.cgi +++ b/httemplate/search/report_prepaid_income.cgi @@ -43,9 +43,8 @@ my( $total, $total_legacy ) = ( 0, 0 ); my @cust_bill_pkg = grep { $_->cust_pkg && $_->cust_pkg->part_pkg->freq !~ /^([01]|\d+[dw])$/ } qsearch( 'cust_bill_pkg', { - 'recur' => { op=>'!=', value=>0 }, - 'edate' => { op=>'>', value=>$now }, - 'duplicate' => '', + 'recur' => { op=>'!=', value=>0 }, + 'edate' => { op=>'>', value=>$now }, }, ); my @cust_pkg = diff --git a/httemplate/search/report_tax.cgi b/httemplate/search/report_tax.cgi index dd6779a61..3d37c47e4 100755 --- a/httemplate/search/report_tax.cgi +++ b/httemplate/search/report_tax.cgi @@ -188,8 +188,7 @@ my $join_pkg = " LEFT JOIN part_pkg USING ( pkgpart ) "; -my $where = "WHERE _date >= $beginning AND _date <= $ending ". - "AND (duplicate IS NULL or duplicate = '')"; +my $where = "WHERE _date >= $beginning AND _date <= $ending "; my @base_param = qw( county county state state country ); if ( $conf->exists('tax-ship_address') ) { -- 2.11.0