RT#34078: Payment History Report / Statement [refactor to not use msg_template]
authorJonathan Prykop <jonathan@freeside.biz>
Thu, 25 Jun 2015 05:51:02 +0000 (00:51 -0500)
committerJonathan Prykop <jonathan@freeside.biz>
Thu, 25 Jun 2015 05:51:02 +0000 (00:51 -0500)
FS/FS/Conf.pm
FS/FS/cust_main.pm
FS/FS/cust_main_Mixin.pm
FS/FS/msg_template.pm
FS/FS/msg_template/InitialData.pm
httemplate/edit/msg_template.html
httemplate/elements/customer-statement.html [new file with mode: 0644]
httemplate/misc/email-customer-statement.html [new file with mode: 0644]
httemplate/misc/email-customers-history.html [deleted file]
httemplate/misc/email-customers.html
httemplate/view/cust_main/menu.html

index 29d993e..238622b 100644 (file)
@@ -2657,13 +2657,6 @@ and customer address. Include units.',
   },
 
   {
-    'key'         => 'payment_history_msgnum',
-    'section'     => 'notification',
-    'description' => 'Template to use for sending payment history to customer',
-    %msg_template_options,
-  },
-
-  {
     'key'         => 'payby',
     'section'     => 'billing',
     'description' => 'Available payment types.',
index b7efa18..f2c2b4a 100644 (file)
@@ -4146,7 +4146,7 @@ I<conf> - optional already-loaded FS::Conf object.
 =cut
 
 # Caution: this gets used by FS::ClientAPI::MyAccount::billing_history,
-# and also payment_history_text, which should both be kept customer-friendly.
+# and also for sending customer statements, which should both be kept customer-friendly.
 # If you add anything that shouldn't be passed on through the API or exposed 
 # to customers, add a new option to include it, don't include it by default
 sub payment_history {
@@ -4268,31 +4268,6 @@ sub payment_history {
   return @out;
 }
 
-=item payment_history_text
-
-Accepts the same options as L</payment_history> and returns those
-results as a string table with fixed-width columns, max width 80 char.
-
-=cut
-
-sub payment_history_text {
-  my $self = shift;
-  my $opt = ref($_[0]) ? $_[0] : { @_ };
-  my $out = sprintf("%-12s",'Date');
-  $out .= sprintf("%11s",'Amount') . '  ';
-  $out .= sprintf("%11s",'Balance') . '  ';
-  $out .= 'Description'; #don't need to pad with spaces
-  $out .= "\n";
-  foreach my $item ($self->payment_history($opt)) {
-    $out .= sprintf("%-10.10s",$$item{'date_pretty'}) . '  ';   #12 width
-    $out .= sprintf("%11.11s",$$item{'amount_pretty'}) . '  ';  #13 width
-    $out .= sprintf("%11.11s",$$item{'balance_pretty'}) . '  '; #13 width
-    $out .= sprintf("%.42s",$$item{'description'});             #max 42 width
-    $out .= "\n";
-  }
-  return $out;
-}
-
 =back
 
 =head1 CLASS METHODS
index 83ca3a2..bdad511 100644 (file)
@@ -394,11 +394,6 @@ HTML body
 
 Text body
 
-=item sub_param
-
-Optional list of parameter hashrefs to be passed
-along to L<FS::msg_template/prepare>.
-
 =back
 
 Returns an error message, or false for success.
@@ -475,8 +470,6 @@ sub email_search_result {
         'cust_main' => $cust_main,
         'object'    => $obj,
       );
-      $message{'sub_param'} = $param->{'sub_param'}
-        if $param->{'sub_param'};
     }
     else {
       my @to = $cust_main->invoicing_list_emailonly;
@@ -554,9 +547,7 @@ sub process_email_search_result {
 
   $param->{'search'} = thaw(decode_base64($param->{'search'}))
     or die "process_email_search_result requires search params.\n";
-  $param->{'sub_param'} = thaw(decode_base64($param->{'sub_param'}))
-    or die "process_email_search_result error decoding sub_param\n"
-      if $param->{'sub_param'};
+
 #  $param->{'payby'} = [ split(/\0/, $param->{'payby'}) ]
 #    unless ref($param->{'payby'});
 
index fe8cbeb..c52b633 100644 (file)
@@ -269,19 +269,7 @@ invoicing_list addresses.  Multiple addresses may be comma-separated.
 
 =item substitutions
 
-A hash reference of additional string substitutions
-
-=item sub_param
-
-A hash reference, keys are the names of existing substitutions,
-values are an addition parameter object to pass to the subroutine
-for that substitution, e.g.
-
-       'sub_param' => {
-         'payment_history' => {
-           'start_date' => 1434764295,
-         },
-       },
+A hash reference of additional substitutions
 
 =back
 
@@ -339,10 +327,7 @@ sub prepare {
       }
       elsif( ref($name) eq 'ARRAY' ) {
         # [ foo => sub { ... } ]
-        my @subparam = ();
-        push(@subparam, $opt{'sub_param'}->{$name->[0]})
-          if $opt{'sub_param'} && $opt{'sub_param'}->{$name->[0]};
-        $hash{$prefix.($name->[0])} = $name->[1]->($obj,@subparam);
+        $hash{$prefix.($name->[0])} = $name->[1]->($obj);
       }
       else {
         warn "bad msg_template substitution: '$name'\n";
@@ -355,10 +340,7 @@ sub prepare {
     $hash{$_} = $opt{substitutions}->{$_} foreach keys %{$opt{substitutions}};
   }
 
-  foreach my $key (keys %hash) {
-    next if $self->no_encode($key);
-    $hash{$key} = encode_entities($_ || '');
-  };
+  $_ = encode_entities($_ || '') foreach values(%hash);
 
   ###
   # clean up template
@@ -527,13 +509,6 @@ my $usage_warning = sub {
 
 #my $conf = new FS::Conf;
 
-# for substitutions that handle their own encoding
-sub no_encode {
-  my $self = shift;
-  my $field = shift;
-  return ($field eq 'payment_history');
-}
-
 #return contexts and fill-in values
 # If you add anything, be sure to add a description in 
 # httemplate/edit/msg_template.html.
@@ -592,12 +567,6 @@ sub substitutions {
       [ selfservice_server_base_url => sub { 
           $conf->config('selfservice_server-base_url') #, shift->agentnum) 
         } ],
-      [ payment_history => sub {
-          my $cust_main = shift;
-          my $param = shift || {};
-          #html works, see no_encode method
-          return '<PRE>' . encode_entities($cust_main->payment_history_text($param)) . '</PRE>';
-        } ],
     ],
     # next_bill_date
     'cust_pkg'  => [qw( 
index 87c407c..a4e27fd 100644 (file)
@@ -21,15 +21,6 @@ If you did not request this password reset, you may safely ignore and delete thi
 END
                       ],
     },
-    { msgname   => 'payment_history_template',
-      mime_type => 'text/html',
-      _conf        => 'payment_history_msgnum',
-      _insert_args => [ subject => '{ $company_name } payment history',
-                        body    => <<'END',
-{ $payment_history }
-END
-                      ],
-    },
   ];
 }
 
index ced98fe..7f38241 100644 (file)
@@ -210,7 +210,6 @@ my %substitutions = (
     '$company_address'=> 'Our company address',
     '$company_phonenum' => 'Our phone number',
     '$selfservice_server_base_url' => 'Base URL of customer self-service',
-    '$payment_history' => 'List of invoices/payments/credits/refunds',
   ],
   'contact' => [ # duplicate this for shipping
     '$name'           => 'Company and contact name',
@@ -323,7 +322,7 @@ my $widget = new HTML::Widgets::SelectLayers(
     my @hints = @{ $substitutions{$section} };
     while(@hints) {
       my $key = shift @hints;
-      $html .= qq!\n<TR><TD STYLE="padding-right: .25em;"><A href="javascript:insertHtml('{$key}')">$key</A></TD>!;
+      $html .= qq!\n<TR><TD><A href="javascript:insertHtml('{$key}')">$key</A></TD>!;
       $html .= "\n<TD>".shift(@hints).'</TD></TR>';
     }
     $html .= "\n</TABLE>";
diff --git a/httemplate/elements/customer-statement.html b/httemplate/elements/customer-statement.html
new file mode 100644 (file)
index 0000000..63c21cb
--- /dev/null
@@ -0,0 +1,45 @@
+<%doc>
+
+Formats customer payment history into a table.
+
+  include('/elements/customer-statement.html',
+    'history' => \@history
+  );
+
+Option 'history' should be of the form returned by $cust_main->payment_history.
+This element might be used directly by selfservice, so it does not (and should not)
+pull data from the database.
+
+</%doc>
+
+% my $style      = 'text-align: left; margin: 0; padding: 0 1em 0 0;';
+% my $moneystyle = 'text-align: right; margin: 0; padding: 0 1em 0 0;';
+
+<TABLE STYLE="margin: 0;" CELLSPACING="0">
+  <TR>
+    <TH STYLE="<% $style %> background: #ff9999;">Date</TH>
+    <TH STYLE="<% $style %> background: #ff9999;">Description</TH>
+    <TH STYLE="<% $moneystyle %> background: #ff9999;">Amount</TH>
+    <TH STYLE="<% $moneystyle %> background: #ff9999;">Balance</TH>
+  </TR>
+
+% my $col1 = "#ffffff";
+% my $col2 = "#dddddd";
+% my $col = $col1;
+% foreach my $item (@{$opt{'history'}}) {
+  <TR>
+    <TD STYLE="<% $style %> background: <% $col %>;"><% $$item{'date_pretty'} %></TD>
+    <TD STYLE="<% $style %> background: <% $col %>;"><% $$item{'description'} %></TD>
+    <TD STYLE="<% $moneystyle %> background: <% $col %>;"><% $$item{'amount_pretty'} %></TD>
+    <TD STYLE="<% $moneystyle %> background: <% $col %>;"><% $$item{'balance_pretty'} %></TD>
+  </TR>
+%   $col = $col eq $col1 ? $col2 : $col1;
+% }
+
+</TABLE>
+
+<%init>
+my %opt = @_;
+
+die "Invalid type for history" unless ref($opt{'history'}) eq 'ARRAY';
+</%init>
diff --git a/httemplate/misc/email-customer-statement.html b/httemplate/misc/email-customer-statement.html
new file mode 100644 (file)
index 0000000..65660f1
--- /dev/null
@@ -0,0 +1,91 @@
+
+ <% include('email-customers.html',
+      'form_action'       => 'email-customer-statement.html',
+      'title'             => 'Send statement to customer',
+      'no_search_fields'  => [ 'start_date', 'end_date' ],
+      'alternate_form'    => $alternate_form,
+      'post_search_hook'  => $post_search_hook,
+    )
+ %>
+
+<%init>
+
+die "access denied"
+  unless $FS::CurrentUser::CurrentUser->access_right('View invoices');
+
+my $alternate_form = sub {
+  # this could maaaybe be a separate element, for cleanliness
+  # but it's really only for use by this page, and it's not overly complicated
+  my $noinit = 0;
+  return join("\n",
+    '<TABLE BORDER="0">',
+    (
+      map {
+        my $label = ucfirst($_);
+        $label =~ s/_/ /;
+        include('/elements/tr-input-date-field.html',{
+          'name' => $_,
+          'value' => $cgi->param($_) || '',
+          'label' => $label,
+          'noinit' => $noinit++
+        });
+      }
+      qw( start_date end_date )
+    ),
+    '</TABLE>',
+    '<INPUT TYPE="hidden" NAME="action" VALUE="preview">',
+    '<INPUT TYPE="submit" VALUE="Preview notice">',
+  );
+};
+
+my $post_search_hook = sub {
+  my %opt = @_;
+  return unless $cgi->param('action') eq 'preview';
+  my $cust_main = qsearchs('cust_main',$opt{'search'})
+    or die "Could not find customer";
+
+  # so that the statement indicates the latest date
+  my $date_format = $opt{'conf'}->config('date_format') || '%m/%d/%Y';
+  $cgi->param('end_date', time2str($date_format, time))
+    unless $cgi->param('end_date');
+
+  # set from/subject/html_body based on date range
+
+  $cgi->param('from',
+    $opt{'conf'}->config('invoice_from')
+  );
+
+  # shortcut for common text
+  my $summary_text  = $cust_main->name_short .
+    ($cgi->param('start_date') ? ' from ' : '') .
+    $cgi->param('start_date') .
+    ($cgi->param('end_date') ? ' through ' : '') .
+    $cgi->param('end_date');
+
+  $cgi->param('subject',
+    $opt{'conf'}->config('company_name') . 
+    ' statement for ' .
+    $summary_text
+  );
+
+  $cgi->param('html_body',
+    '<P>' .
+    $opt{'conf'}->config('company_name') . 
+    ' statement of charges and payments for ' .
+    $summary_text . 
+    "</P>" .
+    include('/elements/customer-statement.html',
+      'history' => [ 
+        $cust_main->payment_history(
+          map {
+            $_ => parse_datetime($cgi->param($_))
+          }
+          qw( start_date end_date ),
+        ),
+      ],
+    )
+  );
+};
+
+</%init>
+
diff --git a/httemplate/misc/email-customers-history.html b/httemplate/misc/email-customers-history.html
deleted file mode 100644 (file)
index 2f9a38d..0000000
+++ /dev/null
@@ -1,51 +0,0 @@
-
- <% include('email-customers.html',
-      'form_action'       => 'email-customers-history.html',
-      'sub_param_process' => $sub_param_process,
-      'alternate_form'    => $alternate_form,
-      'title'             => 'Send payment history',
-    )
- %>
-
-<%init>
-
-my $sub_param_process = sub {
-  my $conf = shift;
-  my %sub_param;
-  foreach my $field ( qw( start_date end_date ) ) {
-    $sub_param{'payment_history'}->{$field} = parse_datetime($cgi->param($field));
-    $cgi->delete($field);
-  }
-  $cgi->param('msgnum',$conf->config('payment_history_msgnum'));
-  return %sub_param;
-};
-
-my $alternate_form = sub {
-  my %sub_param = @_;
-  # this could maaaybe be a separate element, for cleanliness
-  # but it's really only for use by this page, and it's not overly complicated
-  my $noinit = 0;
-  return join("\n",
-    '<TABLE BORDER="0">',
-    (
-      map {
-        my $label = ucfirst($_);
-        $label =~ s/_/ /;
-        include('/elements/tr-input-date-field.html',{
-          'name' => $_,
-          'value' => $sub_param{'payment_history'}->{$_} || '',
-          'label' => $label,
-          'noinit' => $noinit++
-        });
-      }
-      qw( start_date end_date )
-    ),
-    '</TABLE>',
-    '<INPUT TYPE="hidden" NAME="msgnum" VALUE="' . $cgi->param('msgnum') . '">',
-    '<INPUT TYPE="hidden" NAME="action" VALUE="preview">',
-    '<INPUT TYPE="submit" VALUE="Preview notice">',
-  );
-};
-
-</%init>
-
index d1d5ac7..3327303 100644 (file)
@@ -6,18 +6,23 @@ frozen hash in the 'search' cgi param.  Form allows selecting an existing msg_te
 or creating a custom message, and shows a preview of the message before sending.
 If linked to as a popup, include the cgi parameter 'popup' for proper header handling.
 
-This may also be used as an element in other pages, enabling you to pass along
-additional substitution parameters to a message template, with the following options:
+This may also be used as an element in other pages, enabling you to provide an
+alternate initial form while using this for search freezing/thawing and 
+preview/send actions, with the following options:
 
 form_action - the URL to submit the form to
 
-sub_param_process - subroutine to override cgi param values (such as msgnum) 
-and parse/delete additional form fields from the cgi;  should return a %sub_param 
-hash to be passed along for message substitution
+title - the title of the page
 
-alternate_form - an alternate form for template selection/message creation
+no_search_fields - arrayref of additional fields that are not search parameters
 
-title - the title of the page
+alternate_form - subroutine that returns alternate html for the initial form,
+replaces msgnum/from/subject/html_body/action inputs and submit button,
+not used if an action is specified
+
+post_search_hook - sub hook for additional processing after search has been processed from cgi,
+gets passed options 'conf' and 'search' (a reference to the unfrozen %search hash),
+should be used to set msgnum or from/subject/html_body cgi params
 
 </%doc>
 
@@ -35,9 +40,6 @@ title - the title of the page
 %# multi-valued search params.  We are no longer in search context, so we 
 %# pack the search into a Storable string for later use.
 <INPUT TYPE="hidden" NAME="search" VALUE="<% encode_base64(nfreeze(\%search)) %>">
-% if (%sub_param) {
-<INPUT TYPE="hidden" NAME="sub_param" VALUE="<% encode_base64(nfreeze(\%sub_param)) %>">
-% }
 <INPUT TYPE="hidden" NAME="popup" VALUE="<% $popup %>">
 <INPUT TYPE="hidden" NAME="url" VALUE="<% $url | h %>">
 
@@ -47,7 +49,7 @@ title - the title of the page
 
     <% include('/elements/progress-init.html',
                  'OneTrueForm',
-                 [ qw( search table from subject html_body text_body msgnum sub_param ) ],
+                 [ qw( search table from subject html_body text_body msgnum ) ],
                  'process/email-customers.html',
                  $pdest,
               )
@@ -105,7 +107,7 @@ title - the title of the page
 
     </TABLE>
 
-% if ( $cgi->param('action') eq 'preview' ) {
+%   if ( $cgi->param('action') eq 'preview' ) {
 
       <SCRIPT>
         function areyousure(href) {
@@ -119,9 +121,9 @@ title - the title of the page
     
 %   }
 
-% } elsif ($alternate_form) {
+% } elsif ($opt{'alternate_form'}) {
 
-<% $alternate_form %>
+<% &{$opt{'alternate_form'}}() %>
 
 % } else {
 
@@ -174,7 +176,7 @@ Template:
     <INPUT TYPE="hidden" NAME="action" VALUE="preview">
     <INPUT TYPE="submit" VALUE="Preview notice">
 
-% } #end not preview or alternate form
+% } #end not action or alternate form
 
 </FORM>
 
@@ -194,11 +196,11 @@ die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Bulk send customer notices');
 
 my $conf = FS::Conf->new;
+my @no_search_fields = qw( action table from subject html_body text_body popup url );
 
 my $form_action = $opt{'form_action'} || 'email-customers.html';
-my %sub_param = $opt{'sub_param_process'} ? &{$opt{'sub_param_process'}}($conf) : ();
-my $alternate_form = $opt{'alternate_form'} ? &{$opt{'alternate_form'}}(%sub_param) : ();
 my $title = $opt{'title'} || 'Send customer notices';
+push( @no_search_fields, @{$opt{'no_search_fields'}} ) if $opt{'no_search_fields'};
 
 my $table = $cgi->param('table') or die "'table' required";
 my $agent_virt_agentnum = $cgi->param('agent_virt_agentnum') || '';
@@ -214,13 +216,18 @@ if ( $cgi->param('search') ) {
 }
 else {
   %search = $cgi->Vars;
-  delete $search{$_} for qw( action table from subject html_body text_body popup url sub_param );
+  delete $search{$_} for @no_search_fields;
   # FS::$table->search is expected to know which parameters might be 
   # multi-valued, and to accept scalar values for them also.  No good 
   # solution to this since CGI can't tell whether a parameter _might_
   # have had multiple values, only whether it does.
   @search{keys %search} = map { /\0/ ? [ split /\0/, $_ ] : $_ } values %search;
-} 
+}
+
+&{$opt{'post_search_hook'}}(
+  'conf'   => $conf,
+  'search' => \%search,
+) if $opt{'post_search_hook'};
 
 my $num_cust;
 my $from = '';
@@ -238,6 +245,7 @@ my $html_body = $cgi->param('html_body') || '';
 my $msg_template = '';
 
 if ( $cgi->param('action') eq 'preview' ) {
+
   my $sql_query = "FS::$table"->search(\%search);
   my $count_query = delete($sql_query->{'count_query'});
   my $count_sth = dbh->prepare($count_query)
@@ -260,7 +268,6 @@ if ( $cgi->param('action') eq 'preview' ) {
       'cust_main' => $cust,
       'object' => $object,
     );
-    $msgopts{'sub_param'} = \%sub_param if %sub_param; 
     my %message = $msg_template->prepare(%msgopts);
     ($from, $subject, $html_body) = @message{'from', 'subject', 'html_body'};
   }
index ff8937a..6e66ea7 100644 (file)
@@ -483,16 +483,16 @@ my @menu = (
       },
     },
     {
-      label       => 'Email payment history to this customer',
+      label       => 'Email statement to this customer',
       url         => sub {
                       my $cust_main = shift;
                       my $agentnum = $cust_main->agentnum;
-                      'misc/email-customers-history.html?table=cust_main;search_hash='.
+                      'misc/email-customer-statement.html?table=cust_main;search_hash='.
                       'agent_virt_agentnum='.$agentnum.";custnum=$custnum;url=".
                       uri_escape($cgi->self_url);
                      },
       condition   => sub { $invoicing_list_emailonly },
-      acl         => 'Bulk send customer notices',
+      acl         => [ 'View invoices', 'Bulk send customer notices' ],
     },
   ],
   [