Attachment 75417 Details for Bug 11897 – Bug 11897: (QA follow-up) Convert cronscript to use notices

[patch] Bug 11897: (QA follow-up) Convert cronscript to use notices

Bug-11897-QA-follow-up-Convert-cronscript-to-use-n.patch (text/plain), 18.05 KB, created by Martin Renvoize (ashimema) on 2018-05-18 10:46:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2018-05-18 10:46:52 UTC

Size: 18.05 KB

patch

obsolete

>From d1d0cbc182e0ce9f1880de86a0a1b13fe06b45db 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 | 352 +++++++++++++++++++++++------------------
> 1 file changed, 195 insertions(+), 157 deletions(-)
>
>diff --git a/misc/cronjobs/stockrotation.pl b/misc/cronjobs/stockrotation.pl
>index d8914e42d9..2986100ef5 100755
>--- a/misc/cronjobs/stockrotation.pl
>+++ b/misc/cronjobs/stockrotation.pl
>@@ -109,57 +109,54 @@ database updates have been performed.").
> 
> use Modern::Perl;
> use Getopt::Long qw/HelpMessage :config gnu_getopt/;
>-use Mail::Sendmail;
> use C4::Context;
>-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 +174,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,14 +209,16 @@ 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
>---------------------
>+--------------------\n";
>+    $body .= sprintf "
>   Total number of rotas:         %5u
>     Inactive rotas:              %5u
>     Active rotas:                %5u
>@@ -231,36 +230,45 @@ 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,
>+        {
>+            letter => {
>+                title   => 'Stockrotation Report',
>+                content => $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 +290,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 +335,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
>@@ -361,15 +366,14 @@ sub _report_per_branch {
> 
> Return a string containing an overview about $ITEM_SECTION.
> 
>-This helper procedure is only used from within `report_email` and
>-`report_full`.
>+This helper procedure is only used from within `report_full`.
> 
> 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 +383,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,82 +409,116 @@ die.
> =cut
> 
> sub emit {
>-    my ( $params ) = @_;
>-
>-    # REPORT is an arrayref of at least 2 elements:
>-    #   - The header for the report, which will be repeated for each part
>-    #   - a "part" for each report we want to emit
>-    # PARTS are hashrefs:
>-    #   - part->{body}: the body of the report
>-    #   - part->{email_address}: the email address to send the report to
>+    my ($params) = @_;
>+
>+# REPORT is an arrayref of at least 2 elements:
>+#   - The header for the report, which will be repeated for each part
>+#   - a "part" for each report we want to emit
>+# PARTS are hashrefs:
>+#   - part->{status}: a boolean indicating whether the reported part is empty or not
>+#   - part->{email_address}: the email address to send the report to
>+#   - part->{no_branch_email}: a boolean indicating that we are missing a branch email
>+#   - part->{letter}: a GetPreparedLetter hash as returned by the C4::Letters module
>     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};
>-
>-                # 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"',
>-                    }
>-                );
>-
>-                # Send or die.
>-                die($Mail::Sendmail::error)
>-                    unless Mail::Sendmail::sendmail(%mail);
>+    my @emails;
>+    foreach my $part ( @{$parts} ) {
>+
>+        if ( $part->{status} || $params->{send_all} ) {
>+
>+            # We have a report to send, or we want to send even empty
>+            # reports.
>+
>+            # 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
>+                if ( defined($addressee) ) {
>+                    C4::Letters::EnqueueLetter(
>+                        {
>+                            letter                 => $part->{letter},
>+                            to_address             => $addressee,
>+                            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},
>+                            message_transport_type => 'email',
>+                        }
>+                      )
>+                      or warn
>+"can't enqueue letter $part->{letter} for $params->{admin_email}";
>+                }
>+            }
>+            else {
>+                my $email =
>+                  "-------- Email message --------" . "\n\n" . "To: "
>+                  . defined($addressee)               ? $addressee
>+                  : defined( $params->{admin_email} ) ? $params->{admin_email}
>+                  : '' . "\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

Actions: View | Diff | Splinter Review

Attachments on bug 11897: 25887 | 58207 | 58208 | 58209 | 58210 | 58211 | 58212 | 58213 | 58214 | 58215 | 58216 | 58217 | 58218 | 58219 | 58280 | 58281 | 58687 | 58692 | 58693 | 58694 | 58695 | 58696 | 60175 | 60715 | 61825 | 61826 | 61827 | 61828 | 61829 | 61830 | 61831 | 61832 | 61833 | 61834 | 61835 | 61836 | 61837 | 61838 | 61839 | 61885 | 62730 | 62750 | 62751 | 62752 | 62753 | 62754 | 62755 | 62756 | 62757 | 62758 | 62759 | 62760 | 62761 | 62762 | 62763 | 62764 | 62765 | 62766 | 62767 | 63063 | 63064 | 63065 | 63066 | 63067 | 63068 | 63069 | 63070 | 63071 | 63072 | 63073 | 63074 | 63075 | 63076 | 63077 | 63078 | 63079 | 63080 | 63114 | 64850 | 64851 | 65715 | 65716 | 65717 | 65718 | 65719 | 65720 | 65721 | 65722 | 65723 | 65724 | 65725 | 65726 | 65727 | 65728 | 65729 | 65730 | 65731 | 65732 | 65733 | 65734 | 65735 | 65736 | 65737 | 65738 | 65739 | 65740 | 65741 | 65742 | 65743 | 65744 | 65745 | 65746 | 65747 | 65748 | 65749 | 65750 | 65751 | 65752 | 65753 | 65754 | 65755 | 65756 | 65757 | 65758 | 66381 | 70496 | 70497 | 70498 | 70499 | 70500 | 70501 | 70502 | 70503 | 70504 | 70505 | 70506 | 70507 | 70508 | 70509 | 70510 | 70511 | 70512 | 70513 | 70514 | 70515 | 70516 | 70517 | 70518 | 70519 | 70520 | 70521 | 70522 | 70523 | 70524 | 72124 | 72125 | 72126 | 72127 | 72128 | 74003 | 74004 | 74005 | 74006 | 74007 | 74008 | 74010 | 74011 | 74012 | 74013 | 74014 | 74015 | 74016 | 74017 | 74197 | 74198 | 74199 | 74200 | 74201 | 74202 | 74203 | 74204 | 74205 | 74206 | 74319 | 74320 | 74360 | 74459 | 74460 | 74461 | 74462 | 74463 | 74464 | 74465 | 74466 | 74467 | 74468 | 74469 | 74470 | 75409 | 75410 | 75411 | 75412 | 75413 | 75414 | 75415 | 75416 | 75417 | 75418 | 75419 | 75420 | 76121 | 76122 | 76123 | 76124 | 76125 | 76126 | 76127 | 76128 | 76129 | 76130 | 76131 | 76132 | 76133 | 76134 | 76135 | 76144 | 76706 | 76707 | 76708 | 76709 | 76710 | 77623 | 77624 | 77625 | 77626 | 78387 | 78388 | 78389 | 78390 | 78391 | 78392 | 78393 | 78394 | 78404 | 78405 | 78406 | 78407 | 78408 | 79040 | 79041 | 79042 | 79043 | 79044 | 79738 | 79739 | 79740 | 79741 | 79742 | 79746 | 79747 | 79748 | 79749 | 79750 | 79964 | 79965 | 79966 | 79967 | 79968 | 79969 | 79970 | 79971 | 79972 | 79973 | 79974 | 79975 | 80068 | 80087 | 80088 | 80089 | 80090 | 80091 | 80092 | 80093 | 80094 | 80095 | 80096 | 80097 | 80098 | 80099 | 80100 | 80118 | 80260 | 80261 | 80262 | 80263 | 80264 | 80265 | 80266 | 80267 | 80268 | 80269 | 80270 | 80271 | 80272 | 80273 | 80274 | 80275 | 80289 | 80298 | 80299 | 80300