From bc8e1240dd8971ea4b666f2147f45ea91b2d22a7 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 16 Apr 2018 08:58:34 +0100 Subject: [PATCH] Bug 11897: (QA follow-up) Convert cronscript to use notices The cronscript originally used Koha::Email directly, this patch converts it to us the C4::Letters routines. --- misc/cronjobs/stockrotation.pl | 323 +++++++++++++++++++++++------------------ 1 file changed, 180 insertions(+), 143 deletions(-) diff --git a/misc/cronjobs/stockrotation.pl b/misc/cronjobs/stockrotation.pl index d8914e42d9..91fd6f5482 100755 --- a/misc/cronjobs/stockrotation.pl +++ b/misc/cronjobs/stockrotation.pl @@ -115,51 +115,50 @@ use Koha::Email; use Koha::StockRotationRotas; my $admin_email = ''; -my $branch = 0; -my $execute = 0; -my $report = 'full'; -my $send_all = 0; -my $send_email = 0; +my $branch = 0; +my $execute = 0; +my $report = 'full'; +my $send_all = 0; +my $send_email = 0; my $ok = GetOptions( 'admin-email|a=s' => \$admin_email, - 'branchcode|b=s' => sub { - my ($opt_name, $opt_value) = @_; - my $branches = Koha::Libraries->search( - {}, { order_by => { -asc => 'branchname'} } - ); + 'branchcode|b=s' => sub { + my ( $opt_name, $opt_value ) = @_; + my $branches = Koha::Libraries->search( {}, + { order_by => { -asc => 'branchname' } } ); my $brnch = $branches->find($opt_value); - if ( $brnch ) { + if ($brnch) { $branch = $brnch; return $brnch; - } else { + } + else { printf("Option $opt_name should be one of (name -> code):\n"); while ( my $candidate = $branches->next ) { - printf( - " %-40s -> %s\n", $candidate->branchname, - $candidate->branchcode - ); + printf( " %-40s -> %s\n", + $candidate->branchname, $candidate->branchcode ); } - exit 1 + exit 1; } }, - 'execute|x' => \$execute, + 'execute|x' => \$execute, 'report|r=s' => sub { - my ($opt_name, $opt_value) = @_; + my ( $opt_name, $opt_value ) = @_; if ( $opt_value eq 'full' || $opt_value eq 'email' ) { $report = $opt_value; - } else { + } + else { printf("Option $opt_name should be either 'email' or 'full'.\n"); exit 1; } }, - 'send-all|S' => \$send_all, + 'send-all|S' => \$send_all, 'send-email|s' => \$send_email, - 'help|h|?' => sub { HelpMessage } + 'help|h|?' => sub { HelpMessage } ); -exit 1 unless ( $ok ); +exit 1 unless ($ok); -$send_email++ if ( $send_all ); # if we send all, then we must want emails. +$send_email++ if ($send_all); # if we send all, then we must want emails. =head2 Helpers @@ -177,19 +176,19 @@ This procedure WILL mess with your database. =cut sub execute { - my ( $data ) = @_; + my ($data) = @_; # Begin transaction my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; # Carry out db updates - foreach my $item (@{$data->{items}}) { + foreach my $item ( @{ $data->{items} } ) { my $reason = $item->{reason}; if ( $reason eq 'repatriation' ) { $item->{object}->repatriate; - } elsif ( grep { $reason eq $_ } - qw/in-demand advancement initiation/ ) { + } + elsif ( grep { $reason eq $_ } qw/in-demand advancement initiation/ ) { $item->{object}->advance; } } @@ -212,10 +211,11 @@ No data in the database is manipulated by this procedure. =cut sub report_full { - my ( $data ) = @_; + my ($data) = @_; my $header = ""; - my $body = ""; + my $body = ""; + # Summary $header .= sprintf " STOCKROTATION REPORT @@ -231,36 +231,42 @@ STOCKROTATION REPORT Total items to be repatriated: %5u Total items to be advanced: %5u Total items in demand: %5u\n\n", - $data->{sum_rotas}, $data->{rotas_inactive}, $data->{rotas_active}, - $data->{sum_items}, $data->{items_inactive}, $data->{stationary}, - $data->{actionable}, $data->{initiable}, $data->{repatriable}, + $data->{sum_rotas}, $data->{rotas_inactive}, $data->{rotas_active}, + $data->{sum_items}, $data->{items_inactive}, $data->{stationary}, + $data->{actionable}, $data->{initiable}, $data->{repatriable}, $data->{advanceable}, $data->{indemand}; - if (@{$data->{rotas}}) { # Per Rota details + if ( @{ $data->{rotas} } ) { # Per Rota details $body .= sprintf "ROTAS DETAIL\n------------\n\n"; - foreach my $rota (@{$data->{rotas}}) { + foreach my $rota ( @{ $data->{rotas} } ) { $body .= sprintf "Details for %s [%s]:\n", - $rota->{name}, $rota->{id}; - $body .= sprintf "\n Items:"; # Rota item details - if (@{$rota->{items}}) { - $body .= join("", map { _print_item($_) } @{$rota->{items}}); - } else { - $body .= sprintf - "\n No items to be processed for this rota.\n"; + $rota->{name}, $rota->{id}; + $body .= sprintf "\n Items:"; # Rota item details + if ( @{ $rota->{items} } ) { + $body .= + join( "", map { _print_item($_) } @{ $rota->{items} } ); } - $body .= sprintf "\n Log:"; # Rota log details - if (@{$rota->{log}}) { - $body .= join("", map { _print_item($_) } @{$rota->{log}}); - } else { + else { + $body .= + sprintf "\n No items to be processed for this rota.\n"; + } + $body .= sprintf "\n Log:"; # Rota log details + if ( @{ $rota->{log} } ) { + $body .= join( "", map { _print_item($_) } @{ $rota->{log} } ); + } + else { $body .= sprintf "\n No items in log for this rota.\n\n"; } } } - return [ $header, { - body => $body, # The body of the report - status => 1, # We have a meaningful report - no_branch_email => 1, # We don't expect branch email in report - } ]; + return [ + $header, + { + body => $body, # The body of the report + status => 1, # We have a meaningful report + no_branch_email => 1, # We don't expect branch email in report + } + ]; } =head3 report_email @@ -282,33 +288,32 @@ No data in the database is manipulated by this procedure. sub report_email { my ( $data, $branch ) = @_; - my $out = []; + my $out = []; my $header = ""; + # Summary my $branched = $data->{branched}; - my $flag = 0; + my $flag = 0; $header .= sprintf " BRANCH-BASED STOCKROTATION REPORT ---------------------------------\n"; push @{$out}, $header; - if ( $branch ) { # Branch limited report - push @{$out}, _report_per_branch( - $branched->{$branch->branchcode}, $branch->branchcode, - $branch->branchname - ); - } elsif ( $data->{actionable} ) { # Full email report - while ( my ($branchcode_id, $details) = each %{$branched} ) { - push @{$out}, _report_per_branch( - $details, $details->{code}, $details->{name} - ) if ( @{$details->{items}} ); + if ($branch) { # Branch limited report + push @{$out}, _report_per_branch( $branched->{ $branch->branchcode } ); + } + elsif ( $data->{actionable} ) { # Full email report + while ( my ( $branchcode_id, $details ) = each %{$branched} ) { + push @{$out}, _report_per_branch($details) + if ( @{ $details->{items} } ); } - } else { + } + else { push @{$out}, { - body => sprintf " -No actionable items at any libraries.\n\n", # The body of the report - no_branch_email => 1, # We don't expect branch email in report + body => sprintf " +No actionable items at any libraries.\n\n", # The body of the report + no_branch_email => 1, # We don't expect branch email in report }; } return $out; @@ -328,31 +333,29 @@ No data in the database is manipulated by this procedure. =cut sub _report_per_branch { - my ( $per_branch, $branchcode, $branchname ) = @_; + my ($branch) = @_; - my $out = ""; my $status = 0; + if ( $branch && @{ $branch->{items} } ) { + $status = 1; + } - $out .= sprintf "\nStockrotation report for '%s' [%s]:\n", - $branchname || "N/A", $branchcode || "N/A"; - if ( $per_branch && @{$per_branch->{items}} ) { - $out .= sprintf " - Email: %s - Phone: %s - Items:", - $per_branch->{email} || "N/A", $per_branch->{phone} || "N/A"; - $status++; - } else { - $out .= sprintf "No items to be processed for this branch.\n\n"; + if ( + my $letter = C4::Letters::GetPreparedLetter( + module => 'circulation', + letter_code => "SR_SLIP", + message_transport_type => 'email', + substitute => $branch + ) + ) + { + return { + letter => $letter, + email_address => $branch->{email}, + $status + }; } - $out .= join("", map { - _print_item($_) unless $_->{reason} eq 'in-demand' - } @{$per_branch->{items}}); - return { - body => $out, # The body of the report - email_address => $per_branch->{email}, # The branch email address - status => $status, # We may have empty report... - }; + return; } =head3 _print_item @@ -369,7 +372,7 @@ No data in the database is manipulated by this procedure. =cut sub _print_item { - my ( $item ) = @_; + my ($item) = @_; return sprintf " Title: %s Author: %s @@ -379,11 +382,11 @@ sub _print_item { On loan?: %s Status: %s Current Library: %s [%s]\n\n", - $item->{title} || "N/A", $item->{author} || "N/A", - $item->{callnumber} || "N/A", $item->{location} || "N/A", - $item->{barcode} || "N/A", $item->{onloan} ? 'Yes' : 'No', - $item->{reason} || "N/A", $item->{branch}->branchname, - $item->{branch}->branchcode; + $item->{title} || "N/A", $item->{author} || "N/A", + $item->{callnumber} || "N/A", $item->{location} || "N/A", + $item->{barcode} || "N/A", $item->{onloan} ? 'Yes' : 'No', + $item->{reason} || "N/A", $item->{branch}->branchname, + $item->{branch}->branchcode; } =head3 emit @@ -405,7 +408,7 @@ die. =cut sub emit { - my ( $params ) = @_; + my ($params) = @_; # REPORT is an arrayref of at least 2 elements: # - The header for the report, which will be repeated for each part @@ -415,72 +418,106 @@ sub emit { # - part->{email_address}: the email address to send the report to my $report = $params->{report}; my $header = shift @{$report}; - my $parts = $report; + my $parts = $report; - foreach my $part (@{$parts}) { - # The final message is the header + body of this part. - my $msg = $header . $part->{body}; - $msg .= "No database updates have been performed.\n\n" - unless ( $params->{execute} ); - - if ( $params->{send_email} ) { # Only email if emails requested - if ( $part->{status} || $params->{send_all} ) { - # We have a report to send, or we want to send even empty - # reports. - my $message = Koha::Email->new; - - # Collate 'to' addresses - my @to = (); - if ( $part->{email_address} ) { - push @to, $part->{email_address}; - } elsif ( !$part->{no_branch_email} ) { - $msg = "***We tried to send a branch report, but we have no email address for this branch.***\n\n" . $msg; - push @to, C4::Context->preference('KohaAdminEmailAddress') - if ( C4::Context->preference('KohaAdminEmailAddress') ); - } - push @to, $params->{admin_email} if $params->{admin_email}; + my @emails; + foreach my $part ( @{$parts} ) { - # Create email data. - my %mail = $message->create_message_headers( - { - to => join("; ", @to), - subject => "Stockrotation Email Report", - message => Encode::encode("utf8", $msg), - contenttype => 'text/plain; charset="utf8"', - } - ); + if ( $part->{status} || $params->{send_all} ) { + + # We have a report to send, or we want to send even empty + # reports. - # Send or die. - die($Mail::Sendmail::error) - unless Mail::Sendmail::sendmail(%mail); + # Send to branch + my $addressee; + if ( $part->{email_address} ) { + $addressee = $part->{email_address}; } + elsif ( !$part->{no_branch_email} ) { - } else { # Else emit to stdout. - printf $msg; +#push @emails, "***We tried to send a branch report, but we have no email address for this branch.***\n\n"; + $addressee = C4::Context->preference('KohaAdminEmailAddress') + if ( C4::Context->preference('KohaAdminEmailAddress') ); + } + + if ( $params->{send_email} ) { # Only email if emails requested + C4::Letters::EnqueueLetter( + { + letter => $part->{letter}, + to_address => $addressee, + from_address => + C4::Context->preference('KohaAdminEmailAddress'), + message_transport_type => 'email', + } + ) + or warn "can't enqueue letter $part->{letter} for $addressee"; + + # Copy to admin? + if ( $params->{admin_email} ) { + C4::Letters::EnqueueLetter( + { + letter => $part->{letter}, + to_address => $params->{admin_email}, + from_address => + C4::Context->preference('KohaAdminEmailAddress'), + message_transport_type => 'email', + } + ) + or warn +"can't enqueue letter $part->{letter} for $params->{admin_email}"; + } + } + else { + my $email = + "-------- Email message --------" . "\n\n" + . "From: " + . C4::Context->preference('KohaAdminEmailAddress') . "\n" + . "To: " + . $addressee . "\n" + . "Subject: " + . $part->{letter}->{title} . "\n\n" + . $part->{letter}->{content}; + push @emails, $email; + } } } + + # Emit to stdout instead of email? + if ( !$params->{send_email} ) { + + # The final message is the header + body of this part. + my $msg = $header; + $msg .= "No database updates have been performed.\n\n" + unless ( $params->{execute} ); + + # Append email reports to message + $msg .= join( "\n\n", @emails ); + printf $msg; + } } #### Main Code # Compile Stockrotation Report data my $rotas = Koha::StockRotationRotas->search; -my $data = $rotas->investigate; +my $data = $rotas->investigate; # Perform db updates if requested -execute($data) if ( $execute ); +execute($data) if ($execute); # Emit Reports my $out_report = {}; -$out_report = report_email($data, $branch) if $report eq 'email'; -$out_report = report_full($data, $branch) if $report eq 'full'; -emit({ - admin_email => $admin_email, - execute => $execute, - report => $out_report, - send_all => $send_all, - send_email => $send_email, -}); +$out_report = report_email( $data, $branch ) if $report eq 'email'; +$out_report = report_full( $data, $branch ) if $report eq 'full'; +emit( + { + admin_email => $admin_email, + execute => $execute, + report => $out_report, + send_all => $send_all, + send_email => $send_email, + } +); =head1 AUTHOR -- 2.14.2