From bc8e1240dd8971ea4b666f2147f45ea91b2d22a7 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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