detail format refactor, #15535

15 files changed, 772 insertions, 263 deletions

diff --git a/FS/FS/cdr.pm b/FS/FS/cdr.pm
index 31c7c23..842cfab 100644
--- a/FS/FS/cdr.pm
+++ b/FS/FS/cdr.pm
@@ -10,6 +10,7 @@ use Tie::IxHash;
 use Date::Parse;
 use Date::Format;
 use Time::Local;
+use List::Util qw( first min );
 use FS::UID qw( dbh );
 use FS::Conf;
 use FS::Record qw( qsearch qsearchs );
@@ -426,7 +427,9 @@ sub set_status {
 
 Sets the status and rated price.
 
-Available options are: inbound, rated_seconds, rated_minutes, rated_classnum, rated_ratename
+Available options are: inbound, rated_pretty_dst, rated_regionname,
+rated_seconds, rated_minutes, rated_granularity, rated_ratedetailnum,
+rated_classnum, rated_ratename.
 
 If there is an error, returns the error, otherwise returns false.
 
@@ -833,10 +836,12 @@ sub rate_prefix {
 sub rate_upstream_simple {
   my( $self, %opt ) = @_;
 
-  $self->set_status_and_rated_price( 'rated',
-                                     sprintf('%.3f', $self->upstream_price),
-                                     $opt{'svcnum'},
-                                   );
+  $self->set_status_and_rated_price(
+    'rated',
+    sprintf('%.3f', $self->upstream_price),
+    $opt{'svcnum'},
+    'rated_classnum' => $self->calltypenum,
+  );
 }
 
 sub rate_single_price {
@@ -874,10 +879,13 @@ sub rate_single_price {
     sprintf('%.4f', ( $part_pkg->option_cacheable('min_charge') * $charge_min )
                     + 0.0000000001 ); #so 1.00005 rounds to 1.0001
 
-  $self->set_status_and_rated_price( 'rated',
-                                     $charge,
-                                     $opt{'svcnum'},
-                                   );
+  $self->set_status_and_rated_price(
+    'rated',
+    $charge,
+    $opt{'svcnum'},
+    'rated_granularity' => $granularity,
+    'rated_seconds'     => $seconds,
+  );
 
 }
 
@@ -1021,13 +1029,17 @@ my %export_names = (
     'invoice_header' => 'Caller,Date,Time,Number,Destination,Duration,Price',
   },
   'sum_duration' => {
-    'name'           => 'Summary (one line per service, with duration)',
+    'name'           => 'Summary, one line per service',
     'invoice_header' => 'Caller,Rate,Calls,Minutes,Price',
   },
   'sum_count' => {
-    'name'           => 'Summary (one line per service, with count)',
+    'name'           => 'Number of calls, one line per service',
     'invoice_header' => 'Caller,Rate,Messages,Price',
   },
+  'sum_duration_prefix' => {
+    'name'           => 'Summary, one line per destination prefix',
+    'invoice_header' => 'Caller,Rate,Calls,Minutes,Price',
+  },
 );
 
 my %export_formats = ();
@@ -1216,6 +1228,8 @@ as keys (for use with part_pkg::voip_cdr) and "pretty" format names as values.
 
 =cut
 
+# in the future, load this dynamically from detail_format classes
+
 sub invoice_formats {
   map { ($_ => $export_names{$_}->{'name'}) }
     grep { $export_names{$_}->{'invoice_header'} }
diff --git a/FS/FS/cust_bill_pkg.pm b/FS/FS/cust_bill_pkg.pm
index 8d79ed5..7406eda 100644
--- a/FS/FS/cust_bill_pkg.pm
+++ b/FS/FS/cust_bill_pkg.pm
@@ -19,6 +19,8 @@ use FS::cust_bill_pkg_tax_location;
 use FS::cust_bill_pkg_tax_rate_location;
 use FS::cust_tax_adjustment;
 
+use List::Util qw(sum);
+
 @ISA = qw( FS::cust_main_Mixin FS::Record );
 
 $DEBUG = 0;
@@ -147,30 +149,8 @@ sub insert {
 
   if ( $self->get('details') ) {
     foreach my $detail ( @{$self->get('details')} ) {
-      my %hash = ();
-      if ( ref($detail) ) {
-        if ( ref($detail) eq 'ARRAY' ) {
-          #carp "this way sucks, use a hash"; #but more useful/friendly
-          $hash{'format'}      = $detail->[0];
-          $hash{'detail'}      = $detail->[1];
-          $hash{'amount'}      = $detail->[2];
-          $hash{'classnum'}    = $detail->[3];
-          $hash{'phonenum'}    = $detail->[4];
-          $hash{'accountcode'} = $detail->[5];
-          $hash{'startdate'}   = $detail->[6];
-          $hash{'duration'}    = $detail->[7];
-          $hash{'regionname'}  = $detail->[8];
-        } elsif ( ref($detail) eq 'HASH' ) {
-          %hash = %$detail;
-        } else {
-          die "unknow detail type ". ref($detail);
-        }
-      } else {
-        $hash{'detail'} = $detail;
-      }
-      $hash{'billpkgnum'} = $self->billpkgnum;
-      my $cust_bill_pkg_detail = new FS::cust_bill_pkg_detail \%hash;
-      $error = $cust_bill_pkg_detail->insert;
+      $detail->billpkgnum($self->billpkgnum);
+      $error = $detail->insert;
       if ( $error ) {
         $dbh->rollback if $oldAutoCommit;
         return "error inserting cust_bill_pkg_detail: $error";
@@ -351,6 +331,8 @@ sub check {
   ;
   return $error if $error;
 
+  $self->regularize_details;
+
   #if ( $self->pkgnum != 0 ) { #allow unchecked pkgnum 0 for tax! (add to part_pkg?)
   if ( $self->pkgnum > 0 ) { #allow -1 for non-pkg line items and 0 for tax (add to part_pkg?)
     return "Unknown pkgnum ". $self->pkgnum
@@ -363,6 +345,50 @@ sub check {
   $self->SUPER::check;
 }
 
+=item regularize_details
+
+Converts the contents of the 'details' pseudo-field to 
+L<FS::cust_bill_pkg_detail> objects, if they aren't already.
+
+=cut
+
+sub regularize_details {
+  my $self = shift;
+  if ( $self->get('details') ) {
+    foreach my $detail ( @{$self->get('details')} ) {
+      if ( ref($detail) ne 'FS::cust_bill_pkg_detail' ) {
+        # then turn it into one
+        my %hash = ();
+        if ( ! ref($detail) ) {
+          $hash{'detail'} = 'detail';
+        }
+        elsif ( ref($detail) eq 'HASH' ) {
+          %hash = %$detail;
+        }
+        elsif ( ref($detail) eq 'ARRAY' ) {
+          carp "passing invoice details as arrays is deprecated";
+          #carp "this way sucks, use a hash"; #but more useful/friendly
+          $hash{'format'}      = $detail->[0];
+          $hash{'detail'}      = $detail->[1];
+          $hash{'amount'}      = $detail->[2];
+          $hash{'classnum'}    = $detail->[3];
+          $hash{'phonenum'}    = $detail->[4];
+          $hash{'accountcode'} = $detail->[5];
+          $hash{'startdate'}   = $detail->[6];
+          $hash{'duration'}    = $detail->[7];
+          $hash{'regionname'}  = $detail->[8];
+        }
+        else {
+          die "unknown detail type ". ref($detail);
+        }
+        $detail = new FS::cust_bill_pkg_detail \%hash;
+      }
+      $detail->billpkgnum($self->billpkgnum) if $self->billpkgnum;
+    }
+  }
+  return;
+}
+
 =item cust_pkg
 
 Returns the package (see L<FS::cust_pkg>) for this invoice line item.
@@ -863,29 +889,15 @@ usage.
 
 sub usage {
   my( $self, $classnum ) = @_;
+  $self->regularize_details;
 
   if ( $self->get('details') ) {
 
-    my $sum = 0;
-    foreach my $value (
-      map { ref($_) eq 'HASH'
-              ? $_->{'amount'}
-              : $_->[2] 
-          }
-      grep { ref($_) && ( defined($classnum)
-                            ? $classnum eq ( ref($_) eq 'HASH'
-                                               ? $_->{'classnum'}
-                                               : $_->[3]
-                                           )
-                            : 1
-                        )
-           }
+    return sum( 
+      map { $_->amount || 0 }
+      grep { !defined($classnum) or $classnum eq $_->classnum }
       @{ $self->get('details') }
-    ) {
-      $sum += $value if $value;
-    }
-
-    return $sum;
+    );
 
   } else {
 
@@ -911,16 +923,11 @@ details.
 
 sub usage_classes {
   my( $self ) = @_;
+  $self->regularize_details;
 
   if ( $self->get('details') ) {
 
-    my %seen = ();
-    foreach my $detail ( grep { ref($_) } @{$self->get('details')} ) {
-      $seen{ (ref($detail) eq 'HASH'
-               ? $detail->{'classnum'}
-               : $detail->[3]) || ''
-           } = 1;
-    }
+    my %seen = ( map { $_->classnum => 1 } @{ $self->get('details') } );
     keys %seen;
 
   } else {
diff --git a/FS/FS/detail_format.pm b/FS/FS/detail_format.pm
new file mode 100644
index 0000000..144aaa7
--- /dev/null
+++ b/FS/FS/detail_format.pm
@@ -0,0 +1,256 @@
+package FS::detail_format;
+
+use strict;
+use vars qw( $DEBUG );
+use FS::Conf;
+use FS::cdr;
+use FS::cust_bill_pkg_detail;
+use Date::Format qw(time2str);
+use Text::CSV_XS;
+
+my $me = '[FS::detail_format]';
+
+=head1 NAME
+
+FS::detail_format - invoice detail formatter
+
+=head1 DESCRIPTION
+
+An FS::detail_format object is a converter to create invoice details 
+(L<FS::cust_bill_pkg_detail>) from call detail records (L<FS::cdr>)
+or other usage information.  FS::detail_format inherits from nothing.
+
+Subclasses of FS::detail_format represent specific detail formats.
+
+=head1 CLASS METHODS
+
+=over 4
+
+=item new FORMAT, OPTIONS
+
+Returns a new detail formatter.  The FORMAT argument is the name of 
+a subclass.
+
+OPTIONS may contain:
+
+- buffer: an arrayref to store details into.  This may avoid the need for 
+  a large copy operation at the end of processing.  However, since 
+  summary formats will produce nothing until the end of processing, 
+  C<finish> must be called after all CDRs have been appended.
+
+- inbound: a flag telling the formatter to format CDRs for display to 
+  the receiving party, rather than the originator.  In this case, the 
+  L<FS::cdr_termination> object will be fetched and its values used for
+  rated_price, rated_seconds, rated_minutes, and svcnum.  This can be 
+  changed with the C<inbound> method.
+
+=cut
+
+sub new {
+  my $class = shift;
+  if ( $class eq 'FS::detail_format' ) {
+    my $format = shift
+      or die "$me format name required";
+    $class = "FS::detail_format::$format"
+      unless $format =~ /^FS::detail_format::/;
+  }
+  eval "use $class";
+  die "$me error loading $class: $@" if $@;
+  my %opt = @_;
+
+  my $self = { conf => FS::Conf->new,
+               csv  => Text::CSV_XS->new,
+               inbound  => ($opt{'inbound'} ? 1 : 0),
+               buffer   => ($opt{'buffer'} || []),
+             }; 
+  bless $self, $class;
+}
+
+=back
+
+=head1 METHODS
+
+=item inbound VALUE
+
+Set/get the 'inbound' flag.
+
+=cut
+
+sub inbound {
+  my $self = shift;
+  $self->{inbound} = ($_[0] > 0) if (@_);
+  $self->{inbound};
+}
+
+=item append CDRS
+
+Takes any number of call detail records (as L<FS::cdr> objects),
+formats them, and appends them to the internal buffer.
+
+By default, this simply calls C<single_detail> on each CDR in the 
+set.  Subclasses should override C<append> and maybe C<finish> if 
+they do not produce detail lines from CDRs in a 1:1 fashion.
+
+The 'billpkgnum', 'invnum', 'pkgnum', and 'phonenum' fields will 
+be set later.
+
+=cut
+
+sub append {
+  my $self = shift;
+  foreach (@_) {
+    push @{ $self->{buffer} }, $self->single_detail($_);
+  }
+}
+
+=item details
+
+Returns all invoice detail records in the buffer.  This will perform 
+a C<finish> first.  Subclasses generally shouldn't override this.
+
+=cut
+
+sub details {
+  my $self = shift;
+  $self->finish;
+  @{ $self->{buffer} }
+}
+
+=item finish
+
+Ensures that all invoice details are generated given the CDRs that 
+have been appended.  By default, this does nothing.
+
+=cut
+
+sub finish {}
+
+=item header
+
+Returns a header row for the format, as an L<FS::cust_bill_pkg_detail>
+object.  By default this has 'format' = 'C', 'detail' = the value 
+returned by C<header_detail>, and all other fields empty.
+
+This is called after C<finish>, so it can use information from the CDRs.
+
+=cut
+
+sub header {
+  my $self = shift;
+
+  FS::cust_bill_pkg_detail->new(
+    { 'format' => 'C', 'detail' => $self->header_detail }
+  )
+}
+
+=item single_detail CDR
+
+Takes a single CDR and returns an invoice detail to describe it.
+
+By default, this maps the following fields from the CDR:
+
+rated_price       => amount
+rated_classnum    => classnum
+rated_seconds     => duration
+rated_regionname  => regionname
+accountcode       => accountcode
+startdate         => startdate
+
+It then calls C<columns> on the CDR to obtain a list of detail
+columns, formats them as a CSV string, and stores that in the 
+'detail' field.
+
+=cut
+
+sub single_detail {
+  my $self = shift;
+  my $cdr = shift;
+
+  my @columns = $self->columns($cdr);
+  my $status = $self->csv->combine(@columns);
+  die "$me error combining ".$self->csv->error_input."\n"
+    if !$status;
+
+  FS::cust_bill_pkg_detail->new( {
+      'amount'      => $cdr->rated_price,
+      'classnum'    => $cdr->rated_classnum,
+      'duration'    => $cdr->rated_seconds,
+      'regionname'  => $cdr->rated_regionname,
+      'accountcode' => $cdr->accountcode,
+      'startdate'   => $cdr->startdate,
+      'format'      => 'C',
+      'detail'      => $self->csv->string,
+  });
+}
+
+=item columns CDR
+
+Returns a list of CSV columns (to be shown on the invoice) for
+the CDR.  This is the method most subclasses should override.
+
+=cut
+
+sub columns {
+  my $self = shift;
+  die "$me no columns method in ".ref($self);
+}
+
+=item header_detail
+
+Returns the 'detail' field for the header row.  This should 
+probably be a CSV string of column headers for the values returned
+by C<columns>.
+
+=cut
+
+sub header_detail {
+  my $self = shift;
+  die "$me no header_detail method in ".ref($self);
+}
+
+# convenience methods for subclasses
+
+sub conf { $_[0]->{conf} }
+
+sub csv { $_[0]->{csv} }
+
+sub date_format {
+  my $self = shift;
+  $self->{date_format} ||= ($self->conf->config('date_format') || '%m/%d/%Y');
+}
+
+sub money_char {
+  my $self = shift;
+  $self->{money_char} ||= ($self->conf->config('money_char') || '$');
+}
+
+#imitate previous behavior for now
+
+sub duration {
+  my $self = shift;
+  my $cdr = shift;
+  my $object = $self->{inbound} ? $cdr->cdr_termination(1) : $cdr;
+  my $sec = $object->rated_seconds if $object;
+  # XXX termination objects don't have rated_granularity so this may 
+  # result in inbound CDRs being displayed as min/sec when they shouldn't.
+  # Should probably fix this.
+  if ( $cdr->rated_granularity eq '0' ) {
+    '1 call';
+  }
+  elsif ( $cdr->rated_granularity eq '60' ) {
+    sprintf('%dm', ($sec + 59)/60);
+  }
+  else {
+    sprintf('%dm %ds', $sec / 60, $sec % 60);
+  }
+}
+
+sub price {
+  my $self = shift;
+  my $cdr = shift;
+  my $object = $self->{inbound} ? $cdr->cdr_termination(1) : $cdr;
+  my $price = $object->rated_price if $object;
+  length($price) ? $self->money_char . $price : '';
+}
+
+1;
diff --git a/FS/FS/detail_format/accountcode_default.pm b/FS/FS/detail_format/accountcode_default.pm
new file mode 100644
index 0000000..031cc4b
--- /dev/null
+++ b/FS/FS/detail_format/accountcode_default.pm
@@ -0,0 +1,25 @@
+package FS::detail_format::accountcode_default;
+
+use strict;
+use parent qw(FS::detail_format);
+use Date::Format qw(time2str);
+
+sub name { 'Default with accountcode' }
+
+sub header_detail { 'Date,Time,Account,Number,Destination,Duration,Price' }
+
+sub columns {
+  my $self = shift;
+  my $cdr = shift;
+  (
+    time2str($self->date_format, $cdr->startdate),
+    time2str('%r', $cdr->startdate),
+    $cdr->accountcode,
+    ($cdr->rated_pretty_dst || $cdr->dst),
+    $cdr->rated_regionname,
+    $self->duration($cdr),
+    $self->price($cdr),
+  )
+}
+
+1;
diff --git a/FS/FS/detail_format/basic.pm b/FS/FS/detail_format/basic.pm
new file mode 100644
index 0000000..21b5da4
--- /dev/null
+++ b/FS/FS/detail_format/basic.pm
@@ -0,0 +1,22 @@
+package FS::detail_format::basic;
+
+use strict;
+use parent qw(FS::detail_format);
+use Date::Format qw(time2str);
+
+sub name { 'Basic' }
+
+sub header_detail { 'Date/Time,Called Number,Min/Sec,Price' }
+
+sub columns {
+  my $self = shift;
+  my $cdr = shift;
+  (
+    time2str('%d %b - %I:%M %p', $cdr->startdate),
+    $cdr->dst,
+    $self->duration($cdr),
+    $self->price($cdr),
+  )
+}
+
+1;
diff --git a/FS/FS/detail_format/default.pm b/FS/FS/detail_format/default.pm
new file mode 100644
index 0000000..33abb09
--- /dev/null
+++ b/FS/FS/detail_format/default.pm
@@ -0,0 +1,24 @@
+package FS::detail_format::default;
+
+use strict;
+use parent qw(FS::detail_format);
+use Date::Format qw(time2str);
+
+sub name { 'Default' }
+
+sub header_detail { 'Date,Time,Number,Destination,Duration,Price' }
+
+sub columns {
+  my $self = shift;
+  my $cdr = shift;
+  (
+    time2str($self->date_format, $cdr->startdate),
+    time2str('%r', $cdr->startdate),
+    ($cdr->rated_pretty_dst || $cdr->dst),
+    $cdr->rated_regionname,
+    $self->duration($cdr),
+    $self->price($cdr),
+  )
+}
+
+1;
diff --git a/FS/FS/detail_format/description_default.pm b/FS/FS/detail_format/description_default.pm
new file mode 100644
index 0000000..b00983d
--- /dev/null
+++ b/FS/FS/detail_format/description_default.pm
@@ -0,0 +1,25 @@
+package FS::detail_format::description_default;
+
+use strict;
+use parent qw(FS::detail_format);
+use Date::Format qw(time2str);
+
+sub name { 'Default with description field as destination' }
+
+sub header_detail { 'Caller,Date,Time,Number,Destination,Duration,Price' }
+
+sub columns {
+  my $self = shift;
+  my $cdr = shift;
+  (
+    $cdr->src,
+    time2str($self->date_format, $cdr->startdate),
+    time2str('%r', $cdr->startdate),
+    ($cdr->rated_pretty_dst || $cdr->dst),
+    $cdr->description,
+    $self->duration($cdr),
+    $self->price($cdr),
+  )
+}
+
+1;
diff --git a/FS/FS/detail_format/simple.pm b/FS/FS/detail_format/simple.pm
new file mode 100644
index 0000000..cb6b672
--- /dev/null
+++ b/FS/FS/detail_format/simple.pm
@@ -0,0 +1,24 @@
+package FS::detail_format::simple;
+
+use strict;
+use parent qw(FS::detail_format);
+use Date::Format qw(time2str);
+
+sub name { 'Simple' }
+
+sub header_detail { 'Date,Time,Name,Destination,Duration,Price' }
+
+sub columns {
+  my $self = shift;
+  my $cdr = shift;
+  (
+    time2str($self->date_format, $cdr->startdate),
+    time2str('%r', $cdr->startdate),
+    $cdr->userfield,
+    $cdr->dst,
+    $self->duration($cdr),
+    $self->price($cdr),
+  )
+}
+
+1;
diff --git a/FS/FS/detail_format/simple2.pm b/FS/FS/detail_format/simple2.pm
new file mode 100644
index 0000000..5bba18d
--- /dev/null
+++ b/FS/FS/detail_format/simple2.pm
@@ -0,0 +1,25 @@
+package FS::detail_format::simple2;
+
+use strict;
+use parent qw(FS::detail_format);
+use Date::Format qw(time2str);
+
+sub name { 'Simple with source' }
+
+sub header_detail { 'Date,Time,Name,Called From,Destination,Duration,Price' }
+
+sub columns {
+  my $self = shift;
+  my $cdr = shift;
+  (
+    time2str($self->date_format, $cdr->startdate),
+    time2str('%r', $cdr->startdate),
+    $cdr->userfield,
+    $cdr->src,
+    $cdr->dst,
+    $self->duration($cdr),
+    $self->price($cdr),
+  )
+}
+
+1;
diff --git a/FS/FS/detail_format/source_default.pm b/FS/FS/detail_format/source_default.pm
new file mode 100644
index 0000000..26c1146
--- /dev/null
+++ b/FS/FS/detail_format/source_default.pm
@@ -0,0 +1,25 @@
+package FS::detail_format::source_default;
+
+use strict;
+use parent qw(FS::detail_format);
+use Date::Format qw(time2str);
+
+sub name { 'Default with source' }
+
+sub header_detail { 'Caller,Date,Time,Number,Destination,Duration,Price' }
+
+sub columns {
+  my $self = shift;
+  my $cdr = shift;
+  (
+    $cdr->src,
+    time2str($self->date_format, $cdr->startdate),
+    time2str('%r', $cdr->startdate),
+    ($cdr->rated_pretty_dst || $cdr->dst),
+    $cdr->rated_regionname,
+    $self->duration($cdr),
+    $self->price($cdr),
+  )
+}
+
+1;
diff --git a/FS/FS/detail_format/sum_count.pm b/FS/FS/detail_format/sum_count.pm
new file mode 100644
index 0000000..8c395e8
--- /dev/null
+++ b/FS/FS/detail_format/sum_count.pm
@@ -0,0 +1,77 @@
+package FS::detail_format::sum_count;
+
+use strict;
+use vars qw( $DEBUG );
+use parent qw(FS::detail_format);
+use FS::Record qw(qsearchs);
+use FS::cust_svc;
+use FS::svc_Common; # for label
+
+$DEBUG = 0;
+
+sub name { 'Number of calls, one line per service' };
+
+sub header_detail {
+  my $self = shift;
+  if ( $self->{inbound} ) {
+    'Destination,Messages,Price'
+  }
+  else {
+    'Source,Messages,Price'
+  }
+}
+
+sub append {
+  my $self = shift;
+  my $svcnums = ($self->{svcnums} ||= {});
+  foreach my $cdr (@_) {
+    my $object = $self->{inbound} ? $cdr->cdr_termination(1) : $cdr;
+    my $svcnum = $object->svcnum; # yes, $object->svcnum.
+
+    my $subtotal = ($svcnums->{$svcnum} ||=
+      { count => 0, duration => 0, amount => 0 });
+    $subtotal->{count}++;
+    $subtotal->{amount} += $object->rated_price;
+  }
+}
+
+sub finish {
+  my $self = shift;
+  my $svcnums = $self->{svcnums};
+  my $buffer = $self->{buffer};
+  foreach my $svcnum (keys %$svcnums) {
+
+    my $cust_svc = qsearchs('cust_svc', { svcnum => $svcnum })
+      or die "svcnum #$svcnum not found";
+    my $phonenum = $cust_svc->svc_x->label;
+    warn "processing $phonenum\n" if $DEBUG;
+
+    my $subtotal = $svcnums->{$svcnum};
+
+    $self->csv->combine(
+      $phonenum,
+      $subtotal->{count},
+      $self->money_char . sprintf('%.02f',$subtotal->{amount}),
+    );
+
+    warn "adding detail: ".$self->csv->string."\n" if $DEBUG;
+
+    push @$buffer, FS::cust_bill_pkg_detail->new({
+        amount      => $subtotal->{amount},
+        format      => 'C',
+        classnum    => '', #ignored in this format
+        duration    => '',
+        phonenum    => $phonenum,
+        accountcode => '', #ignored in this format
+        startdate   => '', #could use the earliest startdate in the bunch?
+        regionname  => '', #no, we're using prefix instead
+        detail      => $self->csv->string,
+    });
+  } #foreach $svcnum
+
+  # supposedly the compiler is smart enough to do this in place
+  @$buffer = sort { $a->{Hash}->{phonenum} cmp $b->{Hash}->{phonenum} } 
+              @$buffer;
+}
+
+1;
diff --git a/FS/FS/detail_format/sum_duration.pm b/FS/FS/detail_format/sum_duration.pm
new file mode 100644
index 0000000..0b4cedc
--- /dev/null
+++ b/FS/FS/detail_format/sum_duration.pm
@@ -0,0 +1,79 @@
+package FS::detail_format::sum_duration;
+
+use strict;
+use vars qw( $DEBUG );
+use parent qw(FS::detail_format);
+use FS::Record qw(qsearchs);
+use FS::cust_svc;
+use FS::svc_Common; # for label
+
+$DEBUG = 0;
+
+sub name { 'Summary, one line per service' };
+
+sub header_detail {
+  my $self = shift;
+  if ( $self->{inbound} ) {
+    'Destination,Calls,Duration,Price'
+  }
+  else {
+    'Source,Calls,Duration,Price'
+  }
+}
+
+sub append {
+  my $self = shift;
+  my $svcnums = ($self->{svcnums} ||= {});
+  foreach my $cdr (@_) {
+    my $object = $self->{inbound} ? $cdr->cdr_termination(1) : $cdr;
+    my $svcnum = $object->svcnum; # yes, $object->svcnum.
+
+    my $subtotal = ($svcnums->{$svcnum} ||=
+      { count => 0, duration => 0, amount => 0 });
+    $subtotal->{count}++;
+    $subtotal->{duration} += $object->rated_seconds;
+    $subtotal->{amount} += $object->rated_price;
+  }
+}
+
+sub finish {
+  my $self = shift;
+  my $svcnums = $self->{svcnums};
+  my $buffer = $self->{buffer};
+  foreach my $svcnum (keys %$svcnums) {
+
+    my $cust_svc = qsearchs('cust_svc', { svcnum => $svcnum })
+      or die "svcnum #$svcnum not found";
+    my $phonenum = $cust_svc->svc_x->label;
+    warn "processing $phonenum\n" if $DEBUG;
+
+    my $subtotal = $svcnums->{$svcnum};
+
+    $self->csv->combine(
+      $phonenum,
+      $subtotal->{count},
+      int($subtotal->{duration}/60) . ' min',
+      $self->money_char . sprintf('%.02f',$subtotal->{amount}),
+    );
+
+    warn "adding detail: ".$self->csv->string."\n" if $DEBUG;
+
+    push @$buffer, FS::cust_bill_pkg_detail->new({
+        amount      => $subtotal->{amount},
+        format      => 'C',
+        classnum    => '', #ignored in this format
+        duration    => $subtotal->{duration},
+        phonenum    => $phonenum,
+        accountcode => '', #ignored in this format
+        startdate   => '', #could use the earliest startdate in the bunch?
+        regionname  => '', #no, we're using prefix instead
+        detail      => $self->csv->string,
+    });
+  } #foreach $svcnum
+
+  # supposedly the compiler is smart enough to do this in place
+  @$buffer = sort { $a->{Hash}->{phonenum} cmp $b->{Hash}->{phonenum} } 
+              @$buffer;
+}
+
+1;
diff --git a/FS/FS/detail_format/sum_duration_prefix.pm b/FS/FS/detail_format/sum_duration_prefix.pm
new file mode 100644
index 0000000..275aa57
--- /dev/null
+++ b/FS/FS/detail_format/sum_duration_prefix.pm
@@ -0,0 +1,90 @@
+package FS::detail_format::sum_duration_prefix;
+
+use strict;
+use vars qw( $DEBUG );
+use parent qw(FS::detail_format);
+use List::Util qw(sum);
+
+$DEBUG = 0;
+
+my $me = '[sum_duration_prefix]';
+
+sub name { 'Summary, one line per destination prefix' };
+# and also..."rate group"?  what do you call the interstate/intrastate rate 
+# distinction?
+
+sub header_detail {
+  'Destination NPA-NXX,Interstate Calls,Duration,Intrastate Calls,Duration,Price';
+}
+
+my $prefix_length = 6;
+# possibly should use rate_prefix for this, but interstate/intrastate uses 
+# them in a strange way and we are following along
+
+sub append {
+  my $self = shift;
+  my $prefixes = ($self->{prefixes} ||= {});
+  foreach my $cdr (@_) {
+    my $phonenum = $self->{inbound} ? $cdr->src : $cdr->dst;
+    $phonenum =~ /^(\d{$prefix_length})/;
+    my $prefix = $1 || 'other';
+    warn "$me appending ".$cdr->dst." to $prefix\n" if $DEBUG;
+
+    # XXX hardcoded ratenames, not the worst of evils
+    $prefixes->{$prefix} ||= { 
+      Interstate => { count => 0, duration => 0, amount => 0 }, 
+      Intrastate => { count => 0, duration => 0, amount => 0 }, 
+    };
+    my $object = $self->{inbound} ? $cdr->cdr_termination(1) : $cdr;
+    # XXX using $cdr's rated_ratename instead of $object because 
+    # cdr_termination doesn't have one...
+    # but interstate-ness should be symmetric, yes?  if A places an
+    # interstate call to B, then B receives an interstate call from A.
+    my $subtotal = $prefixes->{$prefix}{$cdr->rated_ratename}
+      or die "unknown rated_ratename '" .$cdr->rated_ratename.
+             "' in CDR #".$cdr->acctid."\n";
+    $subtotal->{count}++;
+    $subtotal->{duration} += $object->rated_seconds;
+    $subtotal->{amount} += $object->rated_price;
+  }
+}
+
+sub finish {
+  my $self = shift;
+  my $prefixes = $self->{prefixes};
+  foreach my $prefix (sort { $a cmp $b } keys %$prefixes) {
+
+    warn "processing $prefix\n" if $DEBUG;
+
+    my $ratenames = $prefixes->{$prefix};
+    my @subtotals = ($ratenames->{'Interstate'}, $ratenames->{'Intrastate'});
+    my $total_amount   = sum( map { $_->{'amount'} } @subtotals );
+    my $total_duration = sum( map { $_->{'duration'} } @subtotals );
+    $prefix =~ s/(...)(...)/$1 - $2/;
+
+    $self->csv->combine(
+      $prefix,
+      map({ 
+          ($_->{count} || ''), 
+          ($_->{duration} ? int($_->{duration}/60) . ' min' : '')
+        } @subtotals ),
+      $self->money_char . sprintf('%.02f',$total_amount),
+    );
+
+    warn "adding detail: ".$self->csv->string."\n" if $DEBUG;
+
+    push @{ $self->{buffer} }, FS::cust_bill_pkg_detail->new({
+        amount      => $total_amount,
+        format      => 'C',
+        classnum    => '', #ignored in this format
+        duration    => $total_duration,
+        phonenum    => '', # not divided up per service
+        accountcode => '', #ignored in this format
+        startdate   => '', #could use the earliest startdate in the bunch?
+        regionname  => '', #no, we're using prefix instead
+        detail      => $self->csv->string,
+    });
+  } #foreach $prefix
+}
+
+1;
diff --git a/FS/FS/part_pkg/voip_cdr.pm b/FS/FS/part_pkg/voip_cdr.pm
index ba9e529..ceda0c7 100644
--- a/FS/FS/part_pkg/voip_cdr.pm
+++ b/FS/FS/part_pkg/voip_cdr.pm
@@ -9,6 +9,7 @@ use Text::CSV_XS;
 use FS::Conf;
 use FS::Record qw(qsearchs qsearch);
 use FS::cdr;
+use FS::detail_format;
 #use FS::rate;
 #use FS::rate_prefix;
 #use FS::rate_detail;
@@ -349,9 +350,9 @@ sub calc_usage {
                                  : 'default'
                              );
 
-  my $use_duration = $self->option('use_duration');
+  my $formatter = FS::detail_format->new($output_format, buffer => $details);
 
-  my $csv = new Text::CSV_XS;
+  my $use_duration = $self->option('use_duration');
 
   my($svc_table, $svc_field) = split('\.', $cdr_svc_method);
 
@@ -384,7 +385,7 @@ sub calc_usage {
       );  # $last_bill, $$sdate )
     $options{'by_svcnum'} = 1 if $svc_field eq 'svcnum';
 
-    my @invoice_details_sort;
+    #my @invoice_details_sort;
 
     #first rate any outstanding CDRs not yet rated
     foreach my $cdr (
@@ -404,118 +405,18 @@ sub calc_usage {
 
     #then add details to invoices & get a total
     $options{'status'} = 'rated';
+
     foreach my $cdr (
-      $svc_x->get_cdrs( %options )
+      $svc_x->get_cdrs( %options ) 
     ) {
-
-      my $classnum = '';
-      my @call_details = ();
-      
-      if ( $rating_method eq 'prefix' ) {
-
-        $classnum = $cdr->rated_classnum;
-
-        unless ( $self->sum_usage ) {
-          @call_details = ($cdr->downstream_csv(
-            'format'         => $output_format,
-            'granularity'    => $cdr->rated_granularity, 
-            'seconds'        =>($use_duration ? $cdr->duration : $cdr->billsec),
-            'charge'         => $cdr->rated_price,
-            'pretty_dst'     => $cdr->rated_pretty_dst,
-            'dst_regionname' => $cdr->rated_regionname,
-          ));
-        }
-
-
-      } elsif ( $rating_method eq 'upstream_simple' ) {
-
-        $classnum = $cdr->calltypenum; #? meaningful these days?
-
-        @call_details = ($cdr->downstream_csv(
-          'format' => $output_format,
-          'charge' => $cdr->rated_price,
-        ));
-
-      } elsif ( $rating_method eq 'single_price' ) {
-
-        my $granularity = length($self->option_cacheable('sec_granularity'))
-                            ? $self->option_cacheable('sec_granularity')
-                            : 60;
-
-        @call_details = ($cdr->downstream_csv(
-          'format'      => $output_format,
-          'charge'      => $cdr->rated_price,
-          'seconds'     => ($use_duration ? $cdr->duration : $cdr->billsec),
-          'granularity' => $granularity,
-        ));
-
-      } else {
-        die "don't know how to rate CDRs using method: $rating_method\n";
-      }
-
+      # at this point we officially Do Not Care about the rating method
       $charges += $cdr->rated_price;
-
-      #if ( $cdr->rated_price > 0 ) {
-      # generate a detail record for every call; filter out
-      # $cdr->rated_price == 0 # later.
-      my $call_details;
-      my $phonenum = $svc_x->phonenum;
-
-      if ( scalar(@call_details) == 1 ) {
-        $call_details =
-        { format      => 'C',
-          detail      => $call_details[0],
-          amount      => $cdr->rated_price,
-          classnum    => $classnum,
-          phonenum    => $phonenum,
-          accountcode => $cdr->accountcode,
-          startdate   => $cdr->startdate,
-          duration    => $cdr->rated_seconds,
-          regionname  => $cdr->rated_regionname,
-        };
-      } else { #only used for $rating_method eq 'upstream' now
-        # and for sum_ formats
-        $csv->combine(@call_details);
-        $call_details =
-        { format      => 'C',
-          detail      => $csv->string,
-          amount      => $cdr->rated_price,
-          classnum    => $classnum,
-          phonenum    => $phonenum,
-          accountcode => $cdr->accountcode,
-          startdate   => $cdr->startdate,
-          duration    => $cdr->rated_seconds,
-          regionname  => $cdr->rated_regionname,
-        };
-      }
-      $call_details->{'ratename'} = $cdr->rated_ratename;
-
-      push @invoice_details_sort, [ $call_details, $cdr->calldate_unix ];
-      #} $charge > 0
-
-      my $error = $cdr->set_status('done');
-      die $error if $error; #??
-
+      $formatter->append($cdr);
     }
+  }
 
-    if ( !$self->sum_usage ) {
-      #sort them
-      my @sorted_invoice_details = 
-        sort { @{$a}[1] <=> @{$b}[1] } @invoice_details_sort;
-      foreach my $sorted_call_detail ( @sorted_invoice_details ) {
-        my $d = $sorted_call_detail->[0];
-        push @$details, $d if $d->{amount} > 0;
-      }
-    }
-    else { #$self->sum_usage
-        push @$details, $self->sum_detail($svc_x, \@invoice_details_sort);
-    }
-  } # $cust_svc
-
-  unshift @$details, { format => 'C',
-                       detail => FS::cdr::invoice_header($output_format),
-                     }
-    if @$details && $rating_method ne 'upstream';
+  $formatter->finish; #writes into $details
+  unshift @$details, $formatter->header if @$details;
 
   $charges;
 }
@@ -629,51 +530,6 @@ sub sum_usage {
   $self->option('output_format') =~ /^sum_/;
 }
 
-sub sum_detail {
-  my $self = shift;
-  my $svc_x = shift;
-  my $invoice_details = shift || [];
-  return () if !@$invoice_details;
-  my $details_by_rate = {};
-  # combine the entire set of CDRs
-  foreach ( @$invoice_details ) {
-    my $d = $_->[0];
-    my $sum = $details_by_rate->{ $d->{ratename} } ||= {
-      amount      => 0,
-      format      => 'C',
-      classnum    => '', #XXX
-      duration    => 0,
-      phonenum    => $svc_x->phonenum,
-      accountcode => '', #XXX
-      startdate   => '', #XXX
-      regionname  => '',
-      count       => 0,
-    };
-    $sum->{amount} += $d->{amount};
-    $sum->{duration} += $d->{duration};
-    $sum->{count}++;
-  }
-  my @details;
-  foreach my $ratename ( sort keys(%$details_by_rate) ) {
-    my $sum = $details_by_rate->{$ratename};
-    next if $sum->{count} == 0;
-    my $total_cdr = FS::cdr->new({
-        'billsec' => $sum->{duration},
-        'src'     => $sum->{phonenum},
-      });
-    $sum->{detail} = $total_cdr->downstream_csv(
-      format    => $self->option('output_format'),
-      seconds   => $sum->{duration},
-      charge    => sprintf('%.2f',$sum->{amount}),
-      ratename  => $ratename,
-      phonenum  => $sum->{phonenum},
-      count     => $sum->{count},
-    );
-    push @details, $sum;
-  }
-  @details;
-}
-
 # and whether cust_bill should show a detail line for the service label 
 # (separate from usage details)
 sub hide_svc_detail {
diff --git a/FS/FS/part_pkg/voip_tiered.pm b/FS/FS/part_pkg/voip_tiered.pm
index 492e1f6..e5dcf6d 100644
--- a/FS/FS/part_pkg/voip_tiered.pm
+++ b/FS/FS/part_pkg/voip_tiered.pm
@@ -191,10 +191,9 @@ sub calc_usage {
 
   my $output_format = $self->option('output_format', 'Hush!') || 'default';
 
-  my $csv = new Text::CSV_XS;
+  my $formatter = FS::detail_format->new($output_format, buffer => $details);
 
   my $charges = 0;
-  my @invoice_details_sort;
 
   $options{'status'} = 'processing-tiered';
 
@@ -211,6 +210,8 @@ sub calc_usage {
     foreach my $pass (split('_', $cdr_inout)) {
 
       $options{'inbound'} = ( $pass eq 'inbound' );
+      # tell the formatter what we're sending it
+      $formatter->inbound($options{'inbound'});
 
       foreach my $cdr (
         $svc_x->get_cdrs( %options )
@@ -227,31 +228,6 @@ sub calc_usage {
 
         if ( $charge > 0 ) {
           $charges += $charge;
-
-          my $detail = $self->sum_usage ? '' :
-            $cdr->downstream_csv( 'format'  => $output_format,
-                                  'charge'  => $charge,
-                                  'seconds' => ($use_duration ? 
-                                                  $cdr->duration : 
-                                                  $cdr->billsec),
-                                  'granularity' => $granularity,
-                                );
-
-          my $call_details =
-            { format      => 'C',
-              detail      => $detail,
-              amount      => $charge,
-              #classnum    => $cdr->calltypenum, #classnum
-              #phonenum    => $phonenum, #XXX need this to sort on them
-              accountcode => $cdr->accountcode,
-              startdate   => $cdr->startdate,
-              duration    => $object->rated_seconds,
-            };
-
-           #warn "  adding details on charge to invoice: [ ".
-          #    join(', ', @{$call_details} ). " ]"
-          #  if ( $DEBUG && ref($call_details) );
-          push @invoice_details_sort, [ $call_details, $cdr->calldate_unix ];
         }
 
         my $error = $cdr->set_status_and_rated_price(
@@ -264,32 +240,16 @@ sub calc_usage {
         );
         die $error if $error;
 
+        $formatter->append($cdr);
+
       } # $cdr
 
     } # $pass
 
-    if ( $self->sum_usage ) {
-      # then summarize all accumulated details within this svc_x
-      # and then flush them
-      push @$details, $self->sum_detail($svc_x, \@invoice_details_sort);
-      @invoice_details_sort = ();
-    }
-
   } # $cust_svc
 
-  if ( !$self->sum_usage ) {
-    #sort them
-    my @sorted_invoice_details = 
-      sort { ${$a}[1] <=> ${$b}[1] } @invoice_details_sort;
-    foreach my $sorted_call_detail ( @sorted_invoice_details ) {
-        push @$details, ${$sorted_call_detail}[0];
-    }
-  }
-
-  unshift @$details, { format => 'C',
-                       detail => FS::cdr::invoice_header($output_format),
-                     }
-    if @$details;
+  $formatter->finish;
+  unshift @$details, $formatter->header if @$details;
 
   $charges;
 }