From 66fe31f246e846ed556c283cf7f85dfebb28ebeb Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Fri, 18 Oct 2024 15:17:29 +0000 Subject: [PATCH] Bug 19336: (QA follow-up) Tidy fines.pl Signed-off-by: Kyle M Hall --- misc/cronjobs/fines.pl | 98 ++++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 47 deletions(-) diff --git a/misc/cronjobs/fines.pl b/misc/cronjobs/fines.pl index ef2382e718..97b12c9cf0 100755 --- a/misc/cronjobs/fines.pl +++ b/misc/cronjobs/fines.pl @@ -34,7 +34,7 @@ use Koha::Script -cron; use C4::Context; use C4::Overdues qw( Getoverdues CalcFine UpdateFine ); use Getopt::Long qw( GetOptions ); -use Carp qw( carp croak ); +use Carp qw( carp croak ); use File::Spec; use Try::Tiny qw( catch try ); @@ -56,12 +56,12 @@ my $command_line_options = join(" ",@ARGV); cronlogaction({ info => $command_line_options }); GetOptions( - 'h|help' => \$help, - 'v|verbose' => \$verbose, - 'l|log' => \$log, - 'o|out:s' => \$output_dir, - 'm|maxdays:i' => \$maxdays, - 'i|verifyissue' => \$verify_issue, + 'h|help' => \$help, + 'v|verbose' => \$verbose, + 'l|log' => \$log, + 'o|out:s' => \$output_dir, + 'm|maxdays:i' => \$maxdays, + 'i|verifyissue' => \$verify_issue, 'maxfinesdays=i' => \$maxfinesdays ); my $usage = << 'ENDUSAGE'; @@ -88,33 +88,33 @@ if ($help) { exit; } -my $script_handler = Koha::Script->new({ script => $0 }); +my $script_handler = Koha::Script->new( { script => $0 } ); try { $script_handler->lock_exec; -} -catch { +} catch { my $message = "Skipping execution of $0 ($_)"; print STDERR "$message\n" if $verbose; - cronlogaction({ info => $message }); + cronlogaction( { info => $message } ); exit; }; -my @borrower_fields = - qw(cardnumber categorycode surname firstname email phone address citystate); -my @item_fields = qw(itemnumber barcode date_due); -my @other_fields = qw(days_overdue fine); -my $libname = C4::Context->preference('LibraryName'); -my $control = C4::Context->preference('CircControl'); -my $branch_type = C4::Context->preference('HomeOrHoldingBranch') || 'homebranch'; -my $mode = C4::Context->preference('finesMode'); -my $delim = "\t"; # ? C4::Context->preference('CSVDelimiter') || "\t"; +cronlogaction( { info => $command_line_options } ); + +my @borrower_fields = qw(cardnumber categorycode surname firstname email phone address citystate); +my @item_fields = qw(itemnumber barcode date_due); +my @other_fields = qw(days_overdue fine); +my $libname = C4::Context->preference('LibraryName'); +my $control = C4::Context->preference('CircControl'); +my $branch_type = C4::Context->preference('HomeOrHoldingBranch') || 'homebranch'; +my $mode = C4::Context->preference('finesMode'); +my $delim = "\t"; # ? C4::Context->preference('CSVDelimiter') || "\t"; my %is_holiday; my $today = dt_from_string(); my $filename; -if ($log or $output_dir) { +if ( $log or $output_dir ) { $filename = get_filename($output_dir); } @@ -133,17 +133,16 @@ for my $overdue ( @{$overdues} ) { next if $overdue->{itemlost}; if ( !defined $overdue->{borrowernumber} ) { - carp -"ERROR in Getoverdues : issues.borrowernumber IS NULL. Repair 'issues' table now! Skipping record.\n"; + carp "ERROR in Getoverdues : issues.borrowernumber IS NULL. Repair 'issues' table now! Skipping record.\n"; next; } my $patron = Koha::Patrons->find( $overdue->{borrowernumber} ); my $branchcode = - ( $control eq 'ItemHomeLibrary' ) ? $overdue->{$branch_type} - : ( $control eq 'PatronLibrary' ) ? $patron->branchcode - : $overdue->{branchcode}; + ( $control eq 'ItemHomeLibrary' ) ? $overdue->{$branch_type} + : ( $control eq 'PatronLibrary' ) ? $patron->branchcode + : $overdue->{branchcode}; -# In final case, CircControl must be PickupLibrary. (branchcode comes from issues table here). + # In final case, CircControl must be PickupLibrary. (branchcode comes from issues table here). if ( !exists $is_holiday{$branchcode} ) { $is_holiday{$branchcode} = set_holiday( $branchcode, $today ); } @@ -154,23 +153,28 @@ for my $overdue ( @{$overdues} ) { } ++$counted; - my ( $amount, $unitcounttotal, $unitcount); - if ( defined($maxfinesdays) && $maxfinesdays <= $datedue->delta_days( $today )->in_units( 'days' ) ) { + my ( $amount, $unitcounttotal, $unitcount ); + if ( defined($maxfinesdays) && $maxfinesdays <= $datedue->delta_days($today)->in_units('days') ) { my $itype = $overdue->{itemtype} || $overdue->{itype}; - my $data = Koha::CirculationRules->get_effective_rule( { rule_name => 'overduefinescap', categorycode => $patron->categorycode, itemtype => $itype, branchcode => $branchcode } ); - if ( defined($data->rule_value) ) { - $amount = $data->rule_value; + my $data = Koha::CirculationRules->get_effective_rule( + { + rule_name => 'overduefinescap', categorycode => $patron->categorycode, itemtype => $itype, + branchcode => $branchcode + } + ); + if ( defined( $data->rule_value ) ) { + $amount = $data->rule_value; $unitcount = 'n/a'; - } - else { - print "No overduefinescap defined for {branchcode = $branchcode, itemtype = $itype and categorycode = $patron->categorycode}\n"; + } else { + print + "No overduefinescap defined for {branchcode = $branchcode, itemtype = $itype and categorycode = $patron->categorycode}\n"; } } - if (!$amount) { - ( $amount, $unitcounttotal, $unitcount ) = CalcFine( $overdue, $patron->categorycode, $branchcode, $datedue, $today ); + if ( !$amount ) { + ( $amount, $unitcounttotal, $unitcount ) = + CalcFine( $overdue, $patron->categorycode, $branchcode, $datedue, $today ); } - # Don't update the fine if today is a holiday. # This ensures that dropbox mode will remove the correct amount of fine. if ( @@ -178,12 +182,13 @@ for my $overdue ( @{$overdues} ) { && ( !$is_holiday{$branchcode} || C4::Context->preference('ChargeFinesOnClosedDays') ) && ( $amount && $amount > 0 ) - ) + ) { - if ( $verify_issue ) { + if ($verify_issue) { + # if the issue changed before the script got to it, then pass on it. - my $issue = Koha::Checkouts->find({ issue_id => $overdue->{issue_id} }); - if ( ! $issue or $issue->date_due ne $overdue->{date_due} ) { + my $issue = Koha::Checkouts->find( { issue_id => $overdue->{issue_id} } ); + if ( !$issue or $issue->date_due ne $overdue->{date_due} ) { $counted--; next; } @@ -203,14 +208,13 @@ for my $overdue ( @{$overdues} ) { if ($filename) { my @cells; push @cells, - map { defined $borrower->{$_} ? $borrower->{$_} : q{} } - @borrower_fields; + map { defined $borrower->{$_} ? $borrower->{$_} : q{} } @borrower_fields; push @cells, map { $overdue->{$_} } @item_fields; push @cells, $unitcounttotal, $amount; say {$fh} join $delim, @cells; } } -if ($filename){ +if ($filename) { close $fh; } @@ -231,7 +235,7 @@ Number of Overdue Items: EOM } -cronlogaction({ action => 'End', info => "COMPLETED" }); +cronlogaction( { action => 'End', info => "COMPLETED" } ); sub set_holiday { my ( $branch, $dt ) = @_; @@ -252,7 +256,7 @@ sub get_filename { $name =~ s/\W//; $name .= join q{}, q{_}, $today->ymd(), '.log'; $name = File::Spec->catfile( $directory, $name ); - if ($verbose && $log) { + if ( $verbose && $log ) { say "writing to $name"; } return $name; -- 2.43.0