From 286a56c9c9d4d29bae5e2734f4341d638de9c8e3 Mon Sep 17 00:00:00 2001
From: Mark Wells <mark@freeside.biz>
Date: Thu, 9 Jun 2016 15:56:36 -0700
Subject: [PATCH] remove "failed billing events" report, and add "completed
 with an error" status search, #42637

Conflicts:
	httemplate/elements/menu.html
---
 FS/FS/cust_event.pm                      | 40 +++++++++++++++++++++++++-------
 httemplate/elements/menu.html            |  9 +------
 httemplate/search/report_cust_event.html | 10 ++++----
 3 files changed, 38 insertions(+), 21 deletions(-)

diff --git a/FS/FS/cust_event.pm b/FS/FS/cust_event.pm
index 90c1b907f..09d993e33 100644
--- a/FS/FS/cust_event.pm
+++ b/FS/FS/cust_event.pm
@@ -253,7 +253,13 @@ sub do_event {
     $statustext = "Error running ". $part_event->action. " action: $@";
   } elsif ( $error ) {
     $status = 'done';
-    $statustext = $error;
+    if ( $error eq 'N/A' ) {
+      # archaic way to indicate no-op completion of spool_csv (and maybe
+      # other events)?
+      $self->no_action('Y');
+    } else {
+      $statustext = $error;
+    }
   } else {
     $status = 'done';
   }
@@ -405,17 +411,22 @@ sub search_sql_where {
   if ( @event_status ) {
     my @status;
 
-    my ($done_Y, $done_N);
+    my ($done_Y, $done_N, $done_S);
+    # done_Y: action was taken
+    # done_N: action was not taken
+    # done_S: status message returned
     foreach (@event_status) {
       if ($_ eq 'done_Y') {
         $done_Y = 1;
       } elsif ( $_ eq 'done_N' ) {
         $done_N = 1;
+      } elsif ( $_ eq 'done_S' ) {
+        $done_S = 1;
       } else {
         push @status, $_;
       }
     }
-    if ( $done_Y or $done_N ) {
+    if ( $done_Y or $done_N or $done_S ) {
       push @status, 'done';
     }
     if ( @status ) {
@@ -424,12 +435,23 @@ sub search_sql_where {
                     ')';
     }
 
-    if ( $done_Y and not $done_N ) {
-      push @search, "cust_event.no_action IS NULL";
-    } elsif ( $done_N and not $done_Y ) {
-      push @search, "cust_event.no_action = 'Y'";
-    } # else they're both true, so don't add a constraint, or both false,
-      # and it doesn't matter.
+    # done_S status should include only those where statustext is not null,
+    # and done_Y should include only those where it is.
+    if ( $done_Y and $done_N and $done_S ) {
+      # then not necessary
+    } else {
+      my @done_status;
+      if ( $done_Y ) {
+        push @done_status, "(cust_event.no_action IS NULL AND cust_event.statustext IS NULL)";
+      }
+      if ( $done_N ) {
+        push @done_status, "(cust_event.no_action = 'Y')";
+      }
+      if ( $done_S ) {
+        push @done_status, "(cust_event.no_action IS NULL AND cust_event.statustext IS NOT NULL)";
+      }
+      push @search, join(' OR ', @done_status) if @done_status;
+    }
 
   } # event_status
 
diff --git a/httemplate/elements/menu.html b/httemplate/elements/menu.html
index 84eb511ec..00f4f13b5 100644
--- a/httemplate/elements/menu.html
+++ b/httemplate/elements/menu.html
@@ -325,13 +325,6 @@ $report_employees{'Employee Audit Report'} = [ $fsurl.'search/report_employee_au
   if $curuser->access_right('Employees: Audit Report');
 ;
 
-tie my %report_bill_event, 'Tie::IxHash',
-  'All billing events' => [ $fsurl.'search/report_cust_event.html', 'All billing events for a date range' ],
-  'Billing event errors' => [ $fsurl.'search/report_cust_event.html?failed=1', 'Failed credit cards, processor or printer problems, etc.' ],
-#  'All invoice events' => [ $fsurl.'search/cust_bill_event.html', 'Reports on deprecated, old-style invoice events for a date range' ],
-#  'Invoice event errors' => [ $fsurl.'search/cust_bill_event.html?failed=1', 'Reports on deprecated, old-style events for failed credit cards, processor or printer problems, etc.' ],
-;
-
 tie my %report_payments, 'Tie::IxHash',
   'Payments' => [ $fsurl.'search/report_cust_pay.html', 'Payment report (by type and/or date range)' ],
   'Payment application detail' => [ $fsurl.'search/report_cust_bill_pay_pkg.html', 'Line item application detail' ],
@@ -444,7 +437,7 @@ $report_menu{'Tickets'}        = [ \%report_ticketing, 'Ticket reports' ]
   ;#&& FS::TicketSystem->access_right(\%session, 'Something');
 $report_menu{'Employees'}      =  [ \%report_employees, 'Employee reports'  ]
   if keys %report_employees;
-$report_menu{'Billing events'} =  [ \%report_bill_event, 'Billing events' ]
+$report_menu{'Billing events'} =  [ $fsurl.'search/report_cust_event.html', 'Search billing events by date and status' ]
   if $curuser->access_right('Billing event reports');
 $report_menu{'Financial'}      = [ \%report_financial, 'Financial reports' ]
   if $curuser->access_right('Financial reports') 
diff --git a/httemplate/search/report_cust_event.html b/httemplate/search/report_cust_event.html
index 465a391a8..5b8489bbf 100644
--- a/httemplate/search/report_cust_event.html
+++ b/httemplate/search/report_cust_event.html
@@ -44,16 +44,18 @@
 %     } else {
 
 % # 'initial' is not on here, since nobody needs to see it. also,
-% # 'done_Y' and 'done_N' are shorthand for "done, and no_action
-% # is null" and "done, and no_action = 'Y'".
+% # 'done_Y' = "done, and no_action is null, and statustext is null"
+% # 'done_S' = "done, and no_action is null, and statustext is not null"
+% # 'done_N' = "done, and no_action = 'Y'".
       <& /elements/tr-select.html,
         'label'         => 'Event status',
         'field'         => 'event_status',
         'multiple'      => 1,
         'all_selected'  => 1,
         'size'          => 5,
-        'options'       => [ qw( done_Y done_N failed new locked ) ],
-        'option_labels' => { done_Y => 'Completed',
+        'options'       => [ qw( done_Y done_S done_N failed new locked ) ],
+        'option_labels' => { done_Y => 'Completed normally',
+                             done_S => 'Completed, with an error',
                              done_N => 'Completed, no action taken',
                              failed => 'Failed',
                              new    => 'Not yet processed',
-- 
2.11.0