From 15a4e1674694b76ecc2af87de479aabe370ac03d Mon Sep 17 00:00:00 2001 From: Jonathan Prykop Date: Tue, 22 Sep 2015 01:08:04 -0500 Subject: [PATCH] RT#37908: Convert existing email-sending code to use common interface [removals and switches to FS::Log] --- FS/FS/AccessRight.pm | 2 +- FS/FS/Conf.pm | 39 +-------- FS/FS/Cron/agent_email.pm | 79 ------------------ FS/FS/Cron/backup.pm | 59 ++++++-------- FS/FS/Upgrade.pm | 6 ++ FS/FS/cust_credit.pm | 32 +------- FS/FS/log_context.pm | 2 + FS/FS/pay_batch.pm | 19 ++--- FS/bin/freeside-daily | 4 - FS/bin/freeside-fetch | 93 ---------------------- bin/agent_email | 30 ------- httemplate/misc/delete-cust_credit.cgi | 21 ----- httemplate/view/cust_main/payment_history.html | 2 +- .../view/cust_main/payment_history/credit.html | 11 +-- 14 files changed, 48 insertions(+), 351 deletions(-) delete mode 100644 FS/FS/Cron/agent_email.pm delete mode 100755 FS/bin/freeside-fetch delete mode 100755 bin/agent_email delete mode 100755 httemplate/misc/delete-cust_credit.cgi diff --git a/FS/FS/AccessRight.pm b/FS/FS/AccessRight.pm index 53c7cf622..3f2c0f35d 100644 --- a/FS/FS/AccessRight.pm +++ b/FS/FS/AccessRight.pm @@ -223,7 +223,7 @@ tie my %rights, 'Tie::IxHash', 'Void credit', #NEWER than things marked NEWNEWNEW 'Unvoid credit', #NEWER than things marked NEWNEWNEW { rightname=>'Unapply credit', desc=>'Enable "unapplication" of unclosed credits.' }, #aka unapplycredits - { rightname=>'Delete credit', desc=>'Enable deletion of unclosed credits. Be very careful! Only delete credits that were data-entry errors, not adjustments.' }, #aka. deletecredits Optionally specify one or more comma-separated email addresses to be notified when a credit is deleted. + { rightname=>'Delete credit', desc=>'Enable deletion of unclosed credits. Be very careful! Only delete credits that were data-entry errors, not adjustments.' }, 'View refunds', { rightname=>'Post refund', desc=>'Enable posting of check and cash refunds.' }, 'Post check refund', diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm index 1714c575a..5c4774ab5 100644 --- a/FS/FS/Conf.pm +++ b/FS/FS/Conf.pm @@ -1039,16 +1039,6 @@ my $validate_email = sub { $_[0] =~ }, { - 'key' => 'deletecredits', - #not actually deprecated yet - #'section' => 'deprecated', - #'description' => 'DEPRECATED, now controlled by ACLs. Used to enable deletion of unclosed credits. Be very careful! Only delete credits that were data-entry errors, not adjustments. Optionally specify one or more comma-separated email addresses to be notified when a credit is deleted.', - 'section' => '', - 'description' => 'One or more comma-separated email addresses to be notified when a credit is deleted.', - 'type' => [qw( checkbox text )], - }, - - { 'key' => 'deleterefunds', 'section' => 'billing', 'description' => 'Enable deletion of unclosed refunds. Be very careful! Only delete refunds that were data-entry errors, not adjustments.', @@ -2711,14 +2701,6 @@ and customer address. Include units.', }, { - 'key' => 'dump-email_to', - 'section' => '', - 'description' => "Optional email address to send success/failure message for database dumps.", - 'type' => 'text', - 'validate' => $validate_email, - }, - - { 'key' => 'credit_card-recurring_billing_flag', 'section' => 'billing', 'description' => 'This controls when the system passes the "recurring_billing" flag on credit card transactions. If supported by your processor (and the Business::OnlinePayment processor module), passing the flag indicates this is a recurring transaction and may turn off the CVV requirement. ', @@ -3778,11 +3760,12 @@ and customer address. Include units.', 'select_enum' => [ 'approve', 'decline' ], }, + # replaces batch-errors_to (sent email on error) { - 'key' => 'batch-errors_to', + 'key' => 'batch-errors_not_fatal', 'section' => 'billing', - 'description' => 'Email errors when processing batches to this address. If unspecified, batch processing will stop immediately on error.', - 'type' => 'text', + 'description' => 'If checked, when importing batches from a gateway, item errors will be recorded in the system log without aborting processing. If unchecked, batch processing will fail on error.', + 'type' => 'checkbox', }, #lists could be auto-generated from pay_batch info @@ -4648,13 +4631,6 @@ and customer address. Include units.', }, { - 'key' => 'email_report-subject', - 'section' => '', - 'description' => 'Subject for reports emailed by freeside-fetch. Defaults to "Freeside report".', - 'type' => 'text', - }, - - { 'key' => 'selfservice-head', 'section' => 'self-service', 'description' => 'HTML for the HEAD section of the self-service interface, typically used for LINK stylesheet tags', @@ -5781,13 +5757,6 @@ and customer address. Include units.', }, { - 'key' => 'agent-email_day', - 'section' => '', - 'description' => 'On this day of each month, agents with master customer records containing email addresses will be emailed a list of their customers and balances.', - 'type' => 'text', - }, - - { 'key' => 'report-cust_pay-select_time', 'section' => 'UI', 'description' => 'Enable time selection on payment and refund reports.', diff --git a/FS/FS/Cron/agent_email.pm b/FS/FS/Cron/agent_email.pm deleted file mode 100644 index 6bc1cc643..000000000 --- a/FS/FS/Cron/agent_email.pm +++ /dev/null @@ -1,79 +0,0 @@ -package FS::Cron::agent_email; -use base qw( Exporter ); - -use strict; -use vars qw( @EXPORT_OK $DEBUG ); -use Date::Simple qw(today); -use URI::Escape; -use FS::Mason qw( mason_interps ); -use FS::Conf; -use FS::Misc qw(send_email); -use FS::Record qw(qsearch);# qsearchs); -use FS::agent; - -@EXPORT_OK = qw ( agent_email ); -$DEBUG = 0; - -sub agent_email { - my %opt = @_; - - my $conf = new FS::Conf; - - my $day = $conf->config('agent-email_day') or return; - return unless $day == today->day; - - if ( 1 ) { #XXX if ( %%%RT_ENABLED%%% ) { - require RT; - RT::LoadConfig(); - RT::Init(); - RT::ConnectToDatabase(); - } - - my $from = $conf->invoice_from_full(); - - my $outbuf = '';; - my( $fs_interp, $rt_interp ) = mason_interps('standalone', 'outbuf'=>\$outbuf); - - my $comp = '/search/cust_main.html'; - my %args = ( - 'cust_fields' => 'Cust# | Cust. Status | Customer | Current Balance', - '_type' => 'html-print', - ); - my $query = join('&', map "$_=".uri_escape($args{$_}), keys %args ); - - my $extra_sql = $opt{a} ? " AND agentnum IN ( $opt{a} ) " : ''; - - foreach my $agent ( qsearch({ - 'table' => 'agent', - 'hashref' => { - 'disabled' => '', - 'agent_custnum' => { op=>'!=', value=>'' }, - }, - 'extra_sql' => $extra_sql, - }) - ) - { - - $FS::Mason::Request::QUERY_STRING = $query. '&agentnum='. $agent->agentnum; - $fs_interp->exec($comp); - - my @email = $agent->agent_cust_main->invoicing_list or next; - - warn "emailing ". join(',',@email). " for agent ". $agent->agent. "\n" - if $DEBUG; - send_email( - 'from' => $from, - 'to' => \@email, - 'subject' => 'Customer report', - 'body' => $outbuf, - 'content-type' => 'text/html', - #'content-encoding' - ); - - $outbuf = ''; - - } - -} - -1; diff --git a/FS/FS/Cron/backup.pm b/FS/FS/Cron/backup.pm index cfc8e3624..a192ca90e 100644 --- a/FS/FS/Cron/backup.pm +++ b/FS/FS/Cron/backup.pm @@ -6,7 +6,7 @@ use Exporter; use File::Copy; use Date::Format; use FS::UID qw(driver_name datasrc); -use FS::Misc qw( send_email ); +use FS::Log @ISA = qw( Exporter ); @EXPORT_OK = qw( backup ); @@ -20,7 +20,7 @@ sub backup { my $filename = time2str('%Y%m%d%H%M%S',time); datasrc =~ /dbname=([\w\.]+)$/ - or backup_email_and_die($conf,$filename,"unparsable datasrc ". datasrc); + or backup_log_and_die($filename,"unparsable datasrc ". datasrc); my $database = $1; my $ext; @@ -31,70 +31,61 @@ sub backup { system("mysqldump $database >/var/tmp/$database.sql"); $ext = 'sql'; } else { - backup_email_and_die($conf,$filename,"database dumps not yet supported for ". driver_name); + backup_log_and_die($filename,"database dumps not yet supported for ". driver_name); } chmod 0600, "/var/tmp/$database.$ext"; if ( $conf->config('dump-pgpid') ) { eval 'use GnuPG;'; - backup_email_and_die($conf,$filename,$@) if $@; + backup_log_and_die($filename,$@) if $@; my $gpg = new GnuPG; $gpg->encrypt( plaintext => "/var/tmp/$database.$ext", output => "/var/tmp/$database.gpg", recipient => $conf->config('dump-pgpid'), ); unlink "/var/tmp/$database.$ext" - or backup_email_and_die($conf,$filename,$!); + or backup_log_and_die($filename,$!); chmod 0600, "/var/tmp/$database.gpg"; $ext = 'gpg'; } if ( $localdest ) { copy("/var/tmp/$database.$ext", "$localdest/$filename.$ext") - or backup_email_and_die($conf,$filename,$!); + or backup_log_and_die($filename,$!); chmod 0600, "$localdest/$filename.$ext"; } if ( $scpdest ) { eval "use Net::SCP qw(scp);"; - backup_email_and_die($conf,$filename,$@) if $@; + backup_log_and_die($filename,$@) if $@; scp("/var/tmp/$database.$ext", "$scpdest/$filename.$ext"); } - unlink "/var/tmp/$database.$ext" or backup_email_and_die($conf,$filename,$!); #or just warn? + unlink "/var/tmp/$database.$ext" or backup_log_and_die($filename,$!); #or just warn? - backup_email($conf,$filename); + backup_log($filename); } -#runs backup_email and dies with same error message -sub backup_email_and_die { - my ($conf,$filename,$error) = @_; - backup_email($conf,$filename,$error); - warn "backup_email_and_die called without error message" unless $error; +#runs backup_log and dies with same error message +sub backup_log_and_die { + my ($filename,$error) = @_; + $error = "backup_log_and_die called without error message" unless $error; + backup_log($filename,$error); die $error; } -#checks if email should be sent, sends it -sub backup_email { - my ($conf,$filename,$error) = @_; - my $to = $conf->config('dump-email_to'); - return unless $to; - my $result = $error ? 'FAILED' : 'succeeded'; - my $email_error = send_email( - 'from' => $conf->config('invoice_from'), #or whatever, don't think it matters - 'to' => $to, - 'subject' => 'FREESIDE NOTIFICATION: Backup ' . $result, - 'body' => [ - "This is an automatic message from your Freeside installation.\n", - "Freeside backup $filename $result", - ($error ? " with the following error:\n\n" : "\n"), - ($error || ''), - "\n", - ], - 'msgtype' => 'admin', - ); - warn $email_error if $email_error; +#logs result +sub backup_log { + my ($filename,$error) = @_; + my $result = $error ? "FAILED: $error" : 'succeeded'; + my $message = "backup $filename $result\n"; + my $log = FS::Log->new('Cron::backup'); + if ($error) { + $log->error($message); + } else { + $log->info($message); + } return; } diff --git a/FS/FS/Upgrade.pm b/FS/FS/Upgrade.pm index ffc04bab7..263230b34 100644 --- a/FS/FS/Upgrade.pm +++ b/FS/FS/Upgrade.pm @@ -154,6 +154,12 @@ If you need to continue using the old Form 477 report, turn on the $conf->set('previous_balance-exclude_from_total', ''); } + # switch from specifying an email address to boolean check + if ( $conf->exists('batch-errors_to') ) { + $conf->touch('batch-errors_not_fatal'); + $conf->delete('batch-errors_to'); + } + enable_banned_pay_pad() unless length($conf->config('banned_pay-pad')); } diff --git a/FS/FS/cust_credit.pm b/FS/FS/cust_credit.pm index 544a0e83d..31adebec1 100644 --- a/FS/FS/cust_credit.pm +++ b/FS/FS/cust_credit.pm @@ -9,7 +9,6 @@ use vars qw( $conf $unsuspendauto $me $DEBUG use List::Util qw( min ); use Date::Format; use FS::UID qw( dbh ); -use FS::Misc qw(send_email); use FS::Record qw( qsearch qsearchs dbdef ); use FS::CurrentUser; use FS::cust_pkg; @@ -277,35 +276,6 @@ sub delete { return $error; } - if ( !$opt{void} and $conf->config('deletecredits') ne '' ) { - - my $cust_main = $self->cust_main; - - my $error = send_email( - 'from' => $conf->invoice_from_full($self->cust_main->agentnum), - #invoice_from??? well as good as any - 'to' => $conf->config('deletecredits'), - 'subject' => 'FREESIDE NOTIFICATION: Credit deleted', - 'body' => [ - "This is an automatic message from your Freeside installation\n", - "informing you that the following credit has been deleted:\n", - "\n", - 'crednum: '. $self->crednum. "\n", - 'custnum: '. $self->custnum. - " (". $cust_main->last. ", ". $cust_main->first. ")\n", - 'amount: $'. sprintf("%.2f", $self->amount). "\n", - 'date: '. time2str("%a %b %e %T %Y", $self->_date). "\n", - 'reason: '. $self->reason. "\n", - ], - ); - - if ( $error ) { - $dbh->rollback if $oldAutoCommit; - return "can't send credit deletion notification: $error"; - } - - } - $dbh->commit or die $dbh->errstr if $oldAutoCommit; ''; @@ -415,7 +385,7 @@ sub void { return $error; } - $error = $self->delete(void => 1); # suppress deletecredits warning + $error = $self->delete(); if ( $error ) { $dbh->rollback if $oldAutoCommit; return $error; diff --git a/FS/FS/log_context.pm b/FS/FS/log_context.pm index 403829ac2..bd142471c 100644 --- a/FS/FS/log_context.pm +++ b/FS/FS/log_context.pm @@ -9,7 +9,9 @@ my @contexts = ( qw( bill_and_collect FS::cust_main::Billing::bill_and_collect FS::cust_main::Billing::bill + FS::pay_batch::import_from_gateway Cron::bill + Cron::backup Cron::upload spool_upload daily diff --git a/FS/FS/pay_batch.pm b/FS/FS/pay_batch.pm index 2a522b46e..d7dd7bbe4 100644 --- a/FS/FS/pay_batch.pm +++ b/FS/FS/pay_batch.pm @@ -10,10 +10,10 @@ use Time::Local; use Text::CSV_XS; use Date::Parse qw(str2time); use Business::CreditCard qw(cardtype); -use FS::Misc qw(send_email); # for error notification use FS::Record qw( dbh qsearch qsearchs ); use FS::Conf; use FS::cust_pay; +use FS::Log; =head1 NAME @@ -567,8 +567,8 @@ sub import_from_gateway { ); my @item_errors; - my $mail_on_error = $conf->config('batch-errors_to'); - if ( $mail_on_error ) { + my $errors_not_fatal = $conf->config('batch-errors_not_fatal'); + if ( $errors_not_fatal ) { # construct error trap $proc_opt{'on_parse_error'} = sub { my ($self, $line, $error) = @_; @@ -801,15 +801,10 @@ sub import_from_gateway { "Errors during batch import: ".scalar(@item_errors), @item_errors ); - if ( $mail_on_error ) { - my $subject = "Batch import errors"; #? - my $body = "Import from gateway ".$gateway->label."\n".$error_text; - send_email( - to => $mail_on_error, - from => $conf->invoice_from_full(), - subject => $subject, - body => $body, - ); + if ( $errors_not_fatal ) { + my $message = "Import from gateway ".$gateway->label." errors: ".$error_text; + my $log = FS::Log->new('FS::pay_batch::import_from_gateway'); + $log->error($message); } else { # Bail out. $dbh->rollback if $oldAutoCommit; diff --git a/FS/bin/freeside-daily b/FS/bin/freeside-daily index cb018d1df..6a2daf934 100755 --- a/FS/bin/freeside-daily +++ b/FS/bin/freeside-daily @@ -79,10 +79,6 @@ pay_batch_receive(%opt); use FS::Cron::export_batch qw(export_batch_submit); export_batch_submit(%opt); -#you can skip this by not having the config -use FS::Cron::agent_email qw(agent_email); -agent_email(%opt); - #clears out cacti imports & deletes select database cache files use FS::Cron::cleanup qw( cleanup cleanup_before_backup ); cleanup_before_backup(); diff --git a/FS/bin/freeside-fetch b/FS/bin/freeside-fetch deleted file mode 100755 index c1ab78373..000000000 --- a/FS/bin/freeside-fetch +++ /dev/null @@ -1,93 +0,0 @@ -#!/usr/bin/perl -w - -use strict; -use LWP::UserAgent; -use FS::UID qw(adminsuidsetup); -use FS::Record qw(qsearchs); -use FS::Misc qw(send_email); - -my $user = shift or die &usage; -my $employeelist = shift or die &usage; -my $url = shift or die &usage; -adminsuidsetup $user; - -my @employees = split ',', $employeelist; - -foreach my $employee (@employees) { - - $employee =~ /^(\w+)$/; - - my $access_user = qsearchs( 'access_user', { 'username' => $1 } ); - unless ($access_user) { - warn "Can't find employee $employee... skipping"; - next; - } - - my $email_address = $access_user->option('email_address'); - unless ($email_address) { - warn "No email address for $employee... skipping"; - next; - } - - no warnings 'redefine'; - local *LWP::UserAgent::get_basic_credentials = sub { - return ($access_user->username, $access_user->_password); - }; - - my $ua = new LWP::UserAgent; - $ua->timeout(1800); #30m, some reports can take a while - $ua->agent("FreesideFetcher/0.1 " . $ua->agent); - - my $req = new HTTP::Request GET => $url; - my $res = $ua->request($req); - - my $conf = new FS::Conf; - my $subject = $conf->config('email_report-subject') || 'Freeside report'; - - my %options = ( 'from' => $email_address, - 'to' => $email_address, - 'subject' => $subject, - 'body' => $res->content, - ); - - $options{'content-type'} = $res->content_type - if $res->content_type; - $options{'content-encoding'} = $res->content_encoding - if $res->content_encoding; - - if ($res->is_success) { - send_email %options; - }else{ - warn "fetching $url failed for $employee: " . $res->status_line; - } -} - -sub usage { - die "Usage:\n\n freeside-fetch user employee[,employee ...] url\n\n"; -} - -=head1 NAME - -freeside-fetch - Send a freeside page to a list of employees. - -=head1 SYNOPSIS - - freeside-fetch user employee[,employee ...] url - -=head1 DESCRIPTION - - Fetches a web page specified by url as if employee and emails it to - employee. Useful when run out of cron to send freeside web pages. - - user: Freeside user - - employee: the username of an employee to receive the emailed page. May be a comma separated list - - url: the web page to be received - -=head1 BUGS - - Can leak employee usernames and passwords if requested to access inappropriate urls. - -=cut - diff --git a/bin/agent_email b/bin/agent_email deleted file mode 100755 index 2fe47c4ba..000000000 --- a/bin/agent_email +++ /dev/null @@ -1,30 +0,0 @@ -#!/usr/bin/perl - -use strict; -use Getopt::Std; -use FS::UID qw(adminsuidsetup); - -&untaint_argv; #what it sounds like (eww) -use vars qw(%opt); -getopts("a:", \%opt); - -my $user = shift or die &usage; -adminsuidsetup $user; - -use FS::Cron::agent_email qw(agent_email); -agent_email(%opt); - -### -# subroutines -### - -sub untaint_argv { - foreach $_ ( $[ .. $#ARGV ) { #untaint @ARGV - #$ARGV[$_] =~ /^([\w\-\/]*)$/ || die "Illegal arguement \"$ARGV[$_]\""; - # Date::Parse - $ARGV[$_] =~ /^(.*)$/ || die "Illegal arguement \"$ARGV[$_]\""; - $ARGV[$_]=$1; - } -} - -1; diff --git a/httemplate/misc/delete-cust_credit.cgi b/httemplate/misc/delete-cust_credit.cgi deleted file mode 100755 index 03eb47299..000000000 --- a/httemplate/misc/delete-cust_credit.cgi +++ /dev/null @@ -1,21 +0,0 @@ -% if ( $error ) { -% errorpage($error); -% } else { -<% $cgi->redirect($p. "view/cust_main.cgi?". $custnum) %> -% } -<%init> - -die "access denied" - unless $FS::CurrentUser::CurrentUser->access_right('Delete credit'); - -#untaint crednum -my($query) = $cgi->keywords; -$query =~ /^(\d+)$/ || die "Illegal crednum"; -my $crednum = $1; - -my $cust_credit = qsearchs('cust_credit',{'crednum'=>$crednum}); -my $custnum = $cust_credit->custnum; - -my $error = $cust_credit->delete; - - diff --git a/httemplate/view/cust_main/payment_history.html b/httemplate/view/cust_main/payment_history.html index 1525e9314..c85559543 100644 --- a/httemplate/view/cust_main/payment_history.html +++ b/httemplate/view/cust_main/payment_history.html @@ -231,7 +231,7 @@ my %opt = ( 'Post refund', 'Post check refund', 'Post cash refund ', 'Refund payment', 'Credit card void', 'Echeck void', 'Void payments', 'Unvoid payments', 'Delete payment', 'Unapply payment', - 'Apply credit', 'Delete credit', 'Unapply credit', 'Void credit', 'Unvoid credit', + 'Apply credit', 'Unapply credit', 'Void credit', 'Unvoid credit', 'Delete refund', 'Billing event reports', 'View customer billing events', ) diff --git a/httemplate/view/cust_main/payment_history/credit.html b/httemplate/view/cust_main/payment_history/credit.html index 3eed833d3..db2e5e582 100644 --- a/httemplate/view/cust_main/payment_history/credit.html +++ b/httemplate/view/cust_main/payment_history/credit.html @@ -1,4 +1,4 @@ -<% $credit. ' '. $reason. $desc. $change_pkg. $apply. $delete. $unapply. $void %> +<% $credit. ' '. $reason. $desc. $change_pkg. $apply . $unapply. $void %> <%init> my( $cust_credit, %opt ) = @_; @@ -138,15 +138,6 @@ $void = ' ('. if $cust_credit->closed !~ /^Y/i && $opt{'Void credit'}; -my $delete = ''; -$delete = areyousure_link("${p}misc/delete-cust_credit.cgi?".$cust_credit->crednum, - emt('Are you sure you want to delete this credit?'), - '', - emt('delete') - ) - if $cust_credit->closed !~ /^Y/i - && $opt{'Delete credit'}; - my $unapply = ''; $unapply = areyousure_link("${p}misc/unapply-cust_credit.cgi?".$cust_credit->crednum, emt('Are you sure you want to unapply this credit?'), -- 2.11.0