From e66ed13a39524328ce78de05a2275bda7ed68148 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Tue, 1 Dec 2020 12:50:28 +0000 Subject: [PATCH] Bug 21886: (QA follow-up) Fix indentation, prevent warns, fix parameter, simplify SQL The SQL code was duplicated, I combine them here frombranch needs to take input switch owning to frombranch in second script initialize hash as empty list, not a reference add a newline after printed output if not mailing notices Signed-off-by: Nick Clemens --- C4/Circulation.pm | 33 ++++++++------------------------- misc/cronjobs/advance_notices.pl | 38 +++++++++++++++++++------------------- misc/cronjobs/overdue_notices.pl | 15 +++++++++++---- 3 files changed, 38 insertions(+), 48 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index a0ed814606..2960c30bb9 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2700,31 +2700,14 @@ sub GetUpcomingDueIssues { $params->{'days_in_advance'} = 7 unless exists $params->{'days_in_advance'}; my $dbh = C4::Context->dbh; my $statement; - if($params->{'owning_library'}) { - $statement = <<"END_SQL"; -SELECT * -FROM ( - SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-TO_DAYS( NOW() ) as days_until_due, branches.branchemail - FROM issues - LEFT JOIN items USING (itemnumber) - LEFT OUTER JOIN branches ON (branches.branchcode = items.homebranch) - WHERE returndate is NULL -) tmp -WHERE days_until_due >= 0 AND days_until_due <= ? -END_SQL - } else { - $statement = <<"END_SQL"; -SELECT * -FROM ( - SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-TO_DAYS( NOW() ) as days_until_due, branches.branchemail - FROM issues - LEFT JOIN items USING (itemnumber) - LEFT OUTER JOIN branches USING (branchcode) - WHERE returndate is NULL -) tmp -WHERE days_until_due >= 0 AND days_until_due <= ? -END_SQL - } + $statement = q{ + SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-TO_DAYS( NOW() ) as days_until_due, branches.branchemail + FROM issues + LEFT JOIN items USING (itemnumber) + LEFT JOIN branches ON branches.branchcode = + }; + $statement .= $params->{'owning_library'} ? " items.homebranch " : " issues.branchcode "; + $statement .= " WHERE returndate is NULL AND TO_DAYS( date_due )-TO_DAYS( NOW() ) BETWEEN 0 AND ?"; my @bind_parameters = ( $params->{'days_in_advance'} ); my $sth = $dbh->prepare( $statement ); diff --git a/misc/cronjobs/advance_notices.pl b/misc/cronjobs/advance_notices.pl index c1a95b4870..fb65cd4343 100755 --- a/misc/cronjobs/advance_notices.pl +++ b/misc/cronjobs/advance_notices.pl @@ -214,7 +214,7 @@ GetOptions( 'help|?' => \$help, 'man' => \$man, 'library=s' => \@branchcodes, - 'frombranch' => \$frombranch, + 'frombranch=s' => \$frombranch, 'c' => \$confirm, 'n' => \$nomail, 'm:i' => \$maxdays, @@ -242,7 +242,7 @@ unless ($confirm) { } cronlogaction(); -my %branches = {}; +my %branches = (); if (@branchcodes) { %branches = map { $_ => 1 } @branchcodes; } @@ -304,14 +304,14 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { $due_digest->{ $upcoming->{borrowernumber} }->{count}++; } } else { - my $branchcode; - if($owning_library) { - $branchcode = $upcoming->{'homebranch'}; - } else { - $branchcode = $upcoming->{'branchcode'}; - } - # Skip this DUE if we specify list of libraries and this one is not part of it - next if (@branchcodes && !$branches{$branchcode}); + my $branchcode; + if($owning_library) { + $branchcode = $upcoming->{'homebranch'}; + } else { + $branchcode = $upcoming->{'branchcode'}; + } + # Skip this DUE if we specify list of libraries and this one is not part of it + next if (@branchcodes && !$branches{$branchcode}); my $item = Koha::Items->find( $upcoming->{itemnumber} ); my $letter_type = 'DUE'; @@ -351,14 +351,14 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { $upcoming_digest->{ $upcoming->{borrowernumber} }->{count}++; } } else { - my $branchcode; - if($owning_library) { - $branchcode = $upcoming->{'homebranch'}; - } else { - $branchcode = $upcoming->{'branchcode'}; - } - # Skip this PREDUE if we specify list of libraries and this one is not part of it - next if (@branchcodes && !$branches{$branchcode}); + my $branchcode; + if($owning_library) { + $branchcode = $upcoming->{'homebranch'}; + } else { + $branchcode = $upcoming->{'branchcode'}; + } + # Skip this PREDUE if we specify list of libraries and this one is not part of it + next if (@branchcodes && !$branches{$branchcode}); my $item = Koha::Items->find( $upcoming->{itemnumber} ); my $letter_type = 'PREDUE'; @@ -389,7 +389,7 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { if ($nomail) { for my $letter ( @letters ) { local $, = "\f"; - print $letter->{'content'}; + print $letter->{'content'}."\n"; } } else { diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl index e9e9ac3d34..0a5c721681 100755 --- a/misc/cronjobs/overdue_notices.pl +++ b/misc/cronjobs/overdue_notices.pl @@ -75,7 +75,7 @@ overdue_notices.pl -list-all list all overdues -date emulate overdues run for this date -email type of email that will be used. Can be 'email', 'emailpro' or 'B_email'. Repeatable. - --owning Send notices from item homebranch library instead of issuing library + --frombranch Set the from address for the notice to one of 'item-homebranch' or 'item-issuebranch'. =head1 OPTIONS @@ -184,10 +184,12 @@ use it in order to send overdues on a specific date and not Now. Format: YYYY-MM Allows to specify which type of email will be used. Can be email, emailpro or B_email. Repeatable. -=item B<--owning> +=item B<--frombranch> Use the address information from the item homebranch library instead of the issuing library. +Defaults to 'item-issuebranch' + =back =head1 DESCRIPTION @@ -297,7 +299,7 @@ my $verbose = 0; my $nomail = 0; my $MAX = 90; my $test_mode = 0; -my $owning_library = 0; +my $frombranch = 'item-issuebranch'; my @branchcodes; # Branch(es) passed as parameter my @emails_to_use; # Emails to use for messaging my @emails; # Emails given in command-line parameters @@ -329,7 +331,7 @@ GetOptions( 'borcat=s' => \@myborcat, 'borcatout=s' => \@myborcatout, 'email=s' => \@emails, - 'owning' => \$owning_library, + 'frombranch=s' => \$frombranch, ) or pod2usage(2); pod2usage(1) if $help; pod2usage( -verbose => 2 ) if $man; @@ -339,6 +341,11 @@ if ( defined $csvfilename && $csvfilename =~ /^-/ ) { warn qq(using "$csvfilename" as filename, that seems odd); } +die "--frombranch takes item-homebranch or item-issuebranch only" + unless ( $frombranch eq 'item-issuebranch' + || $frombranch eq 'item-homebranch' ); +my $owning_library = ( $frombranch eq 'item-homebranch' ) ? 1 : 0; + my @overduebranches = C4::Overdues::GetBranchcodesWithOverdueRules(); # Branches with overdue rules my @branches; # Branches passed as parameter with overdue rules my $branchcount = scalar(@overduebranches); -- 2.11.0