From e27122ac9206c9c344584930e525a4a4f6ac3a6a Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Tue, 23 Jan 2024 18:49:53 +0000
Subject: [PATCH] Bug 29507: Tidy

---
 misc/cronjobs/automatic_renewals.pl | 339 ++++++++++++++--------------
 1 file changed, 173 insertions(+), 166 deletions(-)

diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl
index cdb2e1167cc..076ab4b0111 100755
--- a/misc/cronjobs/automatic_renewals.pl
+++ b/misc/cronjobs/automatic_renewals.pl
@@ -88,14 +88,14 @@ use Koha::Checkouts;
 use Koha::Libraries;
 use Koha::Patrons;
 
-my $command_line_options = join(" ",@ARGV);
+my $command_line_options = join( " ", @ARGV );
 
 my ( $help, $send_notices, $verbose, $confirm, $digest_per_branch );
 GetOptions(
-    'h|help' => \$help,
-    's|send-notices' => \$send_notices,
-    'v|verbose'    => \$verbose,
-    'c|confirm'     => \$confirm,
+    'h|help'              => \$help,
+    's|send-notices'      => \$send_notices,
+    'v|verbose'           => \$verbose,
+    'c|confirm'           => \$confirm,
     'b|digest-per-branch' => \$digest_per_branch,
 ) || pod2usage(1);
 
@@ -111,6 +111,7 @@ to 'Never send emails' or 'Follow patron messaging preferences'
 
 END_WARN
 } else {
+
     # If not following cron then:
     # - we should not send if set to never
     # - we will send any notice generated according to preferences if following those
@@ -120,7 +121,7 @@ END_WARN
 # Since advance notice options are not visible in the web-interface
 # unless EnhancedMessagingPreferences is on, let the user know that
 # this script probably isn't going to do much
-if ( ! C4::Context->preference('EnhancedMessagingPreferences') ) {
+if ( !C4::Context->preference('EnhancedMessagingPreferences') ) {
     warn <<'END_WARN';
 
 The "EnhancedMessagingPreferences" syspref is off.
@@ -130,9 +131,9 @@ To change this, edit the "EnhancedMessagingPreferences" syspref.
 END_WARN
 }
 
-cronlogaction({ info => $command_line_options });
+cronlogaction( { info => $command_line_options } );
 
-$verbose = 1 unless $verbose or $confirm;
+$verbose = 1            unless $verbose or $confirm;
 print "Test run only\n" unless $confirm;
 
 print "getting auto renewals\n" if $verbose;
@@ -141,14 +142,12 @@ my @auto_renews = Koha::Checkouts->search(
         auto_renew                   => 1,
         'patron.autorenew_checkouts' => 1,
     },
-    {
-        join => ['patron','item']
-    }
+    { join => [ 'patron', 'item' ] }
 )->as_list;
 print "found " . scalar @auto_renews . " auto renewals\n" if $verbose;
 
 my $cron_options = C4::Context->config('auto_renew_cronjob');
-my $loops = $cron_options ? $cron_options->{parallel_loops_count} // 1 : 1;
+my $loops        = $cron_options ? $cron_options->{parallel_loops_count} // 1 : 1;
 
 # Split the list of issues into chunks to run in parallel
 my @chunks;
@@ -165,20 +164,18 @@ if ( $loops > 1 ) {
         push( @{ $chunks[$i] }, $auto_renew );
     }
     my $pm = Parallel::ForkManager->new($loops);
-  DATA_LOOP:
+DATA_LOOP:
     foreach my $chunk (@chunks) {
         my $pid = $pm->start and next DATA_LOOP;
         _ProcessRenewals($chunk);
         $pm->finish;
     }
     $pm->wait_all_children;
-}
-else {
+} else {
     _ProcessRenewals( \@auto_renews );
 }
 
-cronlogaction({ action => 'End', info => "COMPLETED" });
-
+cronlogaction( { action => 'End', info => "COMPLETED" } );
 
 =head1 METHODS
 
@@ -191,155 +188,167 @@ cronlogaction({ action => 'End', info => "COMPLETED" });
 sub _ProcessRenewals {
     my $auto_renew_issues = shift;
 
-my $renew_digest = {};
-my %report;
-my @item_renewal_ids;
+    my $renew_digest = {};
+    my %report;
+    my @item_renewal_ids;
 
     foreach my $auto_renew (@$auto_renew_issues) {
-    print "examining item '" . $auto_renew->itemnumber . "' to auto renew\n" if $verbose;
+        print "examining item '" . $auto_renew->itemnumber . "' to auto renew\n" if $verbose;
 
-    my ( $borrower_preferences, $wants_messages, $wants_digest ) = ( undef, 0, 0 );
-    if ( $send_notices_pref eq 'preferences' ) {
-        $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences(
-            {
-                borrowernumber => $auto_renew->borrowernumber,
-                message_name   => 'auto_renewals'
-            }
-        );
-        $wants_messages = 1
-            if $borrower_preferences
-            && $borrower_preferences->{transports};
-        $wants_digest = 1
-            if $wants_messages
-            && $borrower_preferences->{wants_digest};
-    } else {    # Preference is never or cron
-        $wants_messages = $send_notices;
-        $wants_digest   = 0;
-    }
-
-    my ( $success, $error, $updated ) = $auto_renew->attempt_auto_renew( { confirm => $confirm } );
-    if ($success) {
-        if ($verbose) {
-            say sprintf "Issue id: %s for borrower: %s and item: %s %s be renewed.",
-                $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber,
-                $confirm ? 'will' : 'would';
-        }
-        if ($confirm) {
-            push @item_renewal_ids, $auto_renew->itemnumber;
-        }
-        push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew
-            if ($wants_messages) && !$wants_digest;
-    } elsif (
-
-        # FIXME Do we need to list every status? Why not simply else?
-           $error eq 'too_many'
-        || $error eq 'on_reserve'
-        || $error eq 'restriction'
-        || $error eq 'overdue'
-        || $error eq 'too_unseen'
-        || $error eq 'auto_account_expired'
-        || $error eq 'auto_too_late'
-        || $error eq 'auto_too_much_oweing'
-        || $error eq 'auto_too_soon'
-        || $error eq 'item_denied_renewal'
-        || $error eq 'item_issued_to_other_patron'
-        )
-    {
-        if ($verbose) {
-            say sprintf "Issue id: %s for borrower: %s and item: %s %s not be renewed. (%s)",
-                $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber,
-                $confirm ? 'will' : 'would', $error;
+        my ( $borrower_preferences, $wants_messages, $wants_digest ) = ( undef, 0, 0 );
+        if ( $send_notices_pref eq 'preferences' ) {
+            $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences(
+                {
+                    borrowernumber => $auto_renew->borrowernumber,
+                    message_name   => 'auto_renewals'
+                }
+            );
+            $wants_messages = 1
+                if $borrower_preferences
+                && $borrower_preferences->{transports};
+            $wants_digest = 1
+                if $wants_messages
+                && $borrower_preferences->{wants_digest};
+        } else {    # Preference is never or cron
+            $wants_messages = $send_notices;
+            $wants_digest   = 0;
         }
-        if ($updated) {
+
+        my ( $success, $error, $updated ) = $auto_renew->attempt_auto_renew( { confirm => $confirm } );
+        if ($success) {
+            if ($verbose) {
+                say sprintf "Issue id: %s for borrower: %s and item: %s %s be renewed.",
+                    $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber,
+                    $confirm ? 'will' : 'would';
+            }
+            if ($confirm) {
+                push @item_renewal_ids, $auto_renew->itemnumber;
+            }
             push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew
-                if $error ne 'auto_too_soon' && ( $wants_messages && !$wants_digest );  # Do not notify if it's too soon
+                if ($wants_messages) && !$wants_digest;
+        } elsif (
+
+            # FIXME Do we need to list every status? Why not simply else?
+               $error eq 'too_many'
+            || $error eq 'on_reserve'
+            || $error eq 'restriction'
+            || $error eq 'overdue'
+            || $error eq 'too_unseen'
+            || $error eq 'auto_account_expired'
+            || $error eq 'auto_too_late'
+            || $error eq 'auto_too_much_oweing'
+            || $error eq 'auto_too_soon'
+            || $error eq 'item_denied_renewal'
+            || $error eq 'item_issued_to_other_patron'
+            )
+        {
+            if ($verbose) {
+                say sprintf "Issue id: %s for borrower: %s and item: %s %s not be renewed. (%s)",
+                    $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber,
+                    $confirm ? 'will' : 'would', $error;
+            }
+            if ($updated) {
+                push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew
+                    if $error ne 'auto_too_soon'
+                    && ( $wants_messages && !$wants_digest );    # Do not notify if it's too soon
+            }
         }
-    }
 
-    if ( $wants_digest ) {
-        # cache this one to process after we've run through all of the items.
-        if ($digest_per_branch) {
-            $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{success}++ if $success;
-            $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{error}++ unless $success || $error eq 'auto_too_soon';
-            $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{results}->{defined $error ? $error : 'auto-renew'}++ ;
-            push @{$renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{issues}}, $auto_renew->itemnumber;
-            $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{updated} = 1 if $updated && (!$error || $error ne 'auto_too_soon');
-        } else {
-            $renew_digest->{ $auto_renew->borrowernumber }->{success} ++ if $success;
-            $renew_digest->{ $auto_renew->borrowernumber }->{error}++ unless $success || $error eq 'auto_too_soon';
-            $renew_digest->{ $auto_renew->borrowernumber }->{results}->{defined $error ? $error : 'auto-renew'}++ ;
-            $renew_digest->{ $auto_renew->borrowernumber }->{updated} = 1 if $updated && (!$error || $error ne 'auto_too_soon');
-            push @{$renew_digest->{ $auto_renew->borrowernumber }->{issues}}, $auto_renew->itemnumber;
+        if ($wants_digest) {
+
+            # cache this one to process after we've run through all of the items.
+            if ($digest_per_branch) {
+                $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{success}++ if $success;
+                $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{error}++
+                    unless $success || $error eq 'auto_too_soon';
+                $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{results}
+                    ->{ defined $error ? $error : 'auto-renew' }++;
+                push @{ $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{issues} },
+                    $auto_renew->itemnumber;
+                $renew_digest->{ $auto_renew->branchcode }->{ $auto_renew->borrowernumber }->{updated} = 1
+                    if $updated && ( !$error || $error ne 'auto_too_soon' );
+            } else {
+                $renew_digest->{ $auto_renew->borrowernumber }->{success}++ if $success;
+                $renew_digest->{ $auto_renew->borrowernumber }->{error}++ unless $success || $error eq 'auto_too_soon';
+                $renew_digest->{ $auto_renew->borrowernumber }->{results}->{ defined $error ? $error : 'auto-renew' }++;
+                $renew_digest->{ $auto_renew->borrowernumber }->{updated} = 1
+                    if $updated && ( !$error || $error ne 'auto_too_soon' );
+                push @{ $renew_digest->{ $auto_renew->borrowernumber }->{issues} }, $auto_renew->itemnumber;
+            }
         }
-    }
 
-}
+    }
 
-if( @item_renewal_ids ){
-    my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX });
-    $indexer->index_records( \@item_renewal_ids, "specialUpdate", "biblioserver" );
-}
+    if (@item_renewal_ids) {
+        my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } );
+        $indexer->index_records( \@item_renewal_ids, "specialUpdate", "biblioserver" );
+    }
 
-if ( $send_notices && $confirm ) {
-    for my $borrowernumber ( keys %report ) {
-        my $patron = Koha::Patrons->find($borrowernumber);
-        my $borrower_preferences =
-            C4::Members::Messaging::GetMessagingPreferences(
+    if ( $send_notices && $confirm ) {
+        for my $borrowernumber ( keys %report ) {
+            my $patron               = Koha::Patrons->find($borrowernumber);
+            my $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences(
                 {
                     borrowernumber => $borrowernumber,
                     message_name   => 'auto_renewals'
                 }
             );
-        for my $issue ( @{ $report{$borrowernumber} } ) {
-            my $item = $issue->item;
-            # Force sending of email and only email if pref is set to "cron"
-            my @transports = $send_notices_pref eq 'preferences' ? keys %{ $borrower_preferences->{'transports'} } : 'email';
-            foreach my $transport ( @transports ) {
-                my $letter = C4::Letters::GetPreparedLetter (
-                    module      => 'circulation',
-                    letter_code => 'AUTO_RENEWALS',
-                    tables      => {
-                        borrowers => $patron->borrowernumber,
-                        issues    => $issue->itemnumber,
-                        items     => $issue->itemnumber,
-                        biblio    => $item->biblionumber,
-                    },
-                    lang => $patron->lang,
-                    message_transport_type => $transport,
-                );
-
-                if ($letter) {
-                    my $library = $patron->library;
-                    my $admin_email_address = $library->from_email_address;
-
-                    C4::Letters::EnqueueLetter(
-                        {
-                            letter                 => $letter,
-                            borrowernumber         => $borrowernumber,
-                            from_address           => $admin_email_address,
-                            message_transport_type => $transport
-                        }
+            for my $issue ( @{ $report{$borrowernumber} } ) {
+                my $item = $issue->item;
+
+                # Force sending of email and only email if pref is set to "cron"
+                my @transports =
+                    $send_notices_pref eq 'preferences' ? keys %{ $borrower_preferences->{'transports'} } : 'email';
+                foreach my $transport (@transports) {
+                    my $letter = C4::Letters::GetPreparedLetter(
+                        module      => 'circulation',
+                        letter_code => 'AUTO_RENEWALS',
+                        tables      => {
+                            borrowers => $patron->borrowernumber,
+                            issues    => $issue->itemnumber,
+                            items     => $issue->itemnumber,
+                            biblio    => $item->biblionumber,
+                        },
+                        lang                   => $patron->lang,
+                        message_transport_type => $transport,
                     );
+
+                    if ($letter) {
+                        my $library             = $patron->library;
+                        my $admin_email_address = $library->from_email_address;
+
+                        C4::Letters::EnqueueLetter(
+                            {
+                                letter                 => $letter,
+                                borrowernumber         => $borrowernumber,
+                                from_address           => $admin_email_address,
+                                message_transport_type => $transport
+                            }
+                        );
+                    }
                 }
             }
         }
-    }
 
-    if ($digest_per_branch) {
-        while (my ($branchcode, $digests) = each %$renew_digest) {
-            send_digests({
-                digests => $digests,
-                branchcode => $branchcode,
-                letter_code => 'AUTO_RENEWALS_DGST',
-            });
+        if ($digest_per_branch) {
+            while ( my ( $branchcode, $digests ) = each %$renew_digest ) {
+                send_digests(
+                    {
+                        digests     => $digests,
+                        branchcode  => $branchcode,
+                        letter_code => 'AUTO_RENEWALS_DGST',
+                    }
+                );
+            }
+        } else {
+            send_digests(
+                {
+                    digests     => $renew_digest,
+                    letter_code => 'AUTO_RENEWALS_DGST',
+                }
+            );
         }
-    } else {
-        send_digests({
-            digests => $renew_digest,
-            letter_code => 'AUTO_RENEWALS_DGST',
-        });
     }
-}
 
 }
 
@@ -371,41 +380,40 @@ String that denote the letter code.
 sub send_digests {
     my $params = shift;
 
-    PATRON: while ( my ( $borrowernumber, $digest ) = each %{$params->{digests}} ) {
+PATRON: while ( my ( $borrowernumber, $digest ) = each %{ $params->{digests} } ) {
         next unless defined $digest->{updated} && $digest->{updated} == 1;
-        my $borrower_preferences =
-            C4::Members::Messaging::GetMessagingPreferences(
-                {
-                    borrowernumber => $borrowernumber,
-                    message_name   => 'auto_renewals'
-                }
-            );
+        my $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences(
+            {
+                borrowernumber => $borrowernumber,
+                message_name   => 'auto_renewals'
+            }
+        );
 
-        next PATRON unless $borrower_preferences; # how could this happen?
+        next PATRON unless $borrower_preferences;    # how could this happen?
 
-        my $patron = Koha::Patrons->find( $borrowernumber );
+        my $patron = Koha::Patrons->find($borrowernumber);
         my $branchcode;
         if ( defined $params->{branchcode} ) {
             $branchcode = $params->{branchcode};
         } else {
             $branchcode = $patron->branchcode;
         }
-        my $library = Koha::Libraries->find( $branchcode );
+        my $library      = Koha::Libraries->find($branchcode);
         my $from_address = $library->from_email_address;
 
         foreach my $transport ( keys %{ $borrower_preferences->{'transports'} } ) {
-            my $letter = C4::Letters::GetPreparedLetter (
-                module => 'circulation',
+            my $letter = C4::Letters::GetPreparedLetter(
+                module      => 'circulation',
                 letter_code => $params->{letter_code},
-                branchcode => $branchcode,
-                lang => $patron->lang,
-                substitute => {
-                    error => $digest->{error}||0,
-                    success => $digest->{success}||0,
+                branchcode  => $branchcode,
+                lang        => $patron->lang,
+                substitute  => {
+                    error   => $digest->{error}   || 0,
+                    success => $digest->{success} || 0,
                     results => $digest->{results},
                 },
-                loops => { issues => \@{$digest->{issues}} },
-                tables      => {
+                loops  => { issues => \@{ $digest->{issues} } },
+                tables => {
                     borrowers => $patron->borrowernumber,
                 },
                 message_transport_type => $transport,
@@ -420,10 +428,9 @@ sub send_digests {
                         message_transport_type => $transport
                     }
                 );
-            }
-            else {
+            } else {
                 warn
-"no letter of type '$params->{letter_code}' found for borrowernumber $borrowernumber. Please see sample_notices.sql";
+                    "no letter of type '$params->{letter_code}' found for borrowernumber $borrowernumber. Please see sample_notices.sql";
             }
 
         }
-- 
2.30.2