From 3911be4b06773767c81b1abe6a49620751832dd6 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 23 Oct 2013 10:13:50 +0200 Subject: [PATCH] [PASSED QA] Bug 11120: FIX the --date option for overdue_notices cronjob Bug 7447 introduces the --date option for overdue notices. This option has never worked: the code is waiting for a value but the option is defined as a boolean. This patch fixes the option and change the way to calculate the range of dates. This range is now managed in Perl instead of in the SQL query. To do it in Perl allows to build dates simply using the DateTime and DateTime::Duration modules. To test this patch you should have a DB with a lot of overdues, (I tested on a DB with 512 overdues). A test plan could be: 1/ Dump your message_queue table 2/ Verify the number of overdues in the database before applying the patch: mysql> DELETE FROM message_queue; perl misc/cronjobs/overdue_notices.pl -v -t (the triggered option will generate overdue for today) mysql> SELECT COUNT(*) FROM message_queue; Note this value 2A mysql> DELETE FROM message_queue; perl misc/cronjobs/overdue_notices.pl -v mysql> SELECT COUNT(*) FROM message_queue; Note this value 2B 2/ Apply the patch 4/ Verify the number of overdues generated by the patched script: mysql> DELETE FROM message_queue; perl misc/cronjobs/overdue_notices.pl -v -t mysql> SELECT COUNT(*) FROM message_queue; Note this value 4A mysql> DELETE FROM message_queue; perl misc/cronjobs/overdue_notices.pl -v mysql> SELECT COUNT(*) FROM message_queue; Note this value 4B mysql> DELETE FROM message_queue; # The date should be defined depending your dateformat preference # and should be the date of the current day perl misc/cronjobs/overdue_notices.pl -v -t --date="YYYY-MM-DD" mysql> SELECT COUNT(*) FROM message_queue; Note this value 4C mysql> DELETE FROM message_queue; # The date should be defined depending your dateformat preference # and should be the date of the current day perl misc/cronjobs/overdue_notices.pl -v --date="YYYY-MM-DD" mysql> SELECT COUNT(*) FROM message_queue; Note this value 4D 5/ Compare the values: All values generated with the -t options should be equals. Same for values without the -t options. => 2A == 4A == 4C and 2B == 4B == 4D 6/ Go back to a normal activity for 3 days or manually change the date_due for issues in the DB: mysql> update issues SET date_due = DATE_SUB(date_due, INTERVAL 3 DAY); Do again step 4C and 4D with a date equals to today - 3 days. Values should be the same as 4C and 4D. Signed-off-by: Jonathan Druart Signed-off-by: Katrin Fischer Tested with my own test data, checked generating overdues with and without the --date option. All worked as expected. --- misc/cronjobs/overdue_notices.pl | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl index 4d05ff7..61d18cd 100755 --- a/misc/cronjobs/overdue_notices.pl +++ b/misc/cronjobs/overdue_notices.pl @@ -34,6 +34,8 @@ use Pod::Usage; use Text::CSV_XS; use Locale::Currency::Format 1.28; use Encode; +use DateTime; +use DateTime::Duration; use C4::Context; use C4::Dates qw/format_date/; @@ -41,6 +43,7 @@ use C4::Debug; use C4::Letters; use C4::Overdues qw(GetFine GetOverdueMessageTransportTypes); use C4::Budgets qw(GetCurrency); +use Koha::DateUtils; use Koha::Borrower::Debarments qw(AddUniqueDebarment); use Koha::DateUtils; @@ -290,7 +293,7 @@ my $listall = 0; my $itemscontent = join( ',', qw( date_due title barcode author itemnumber ) ); my @myborcat; my @myborcatout; -my $date; +my ( $date_input, $today ); GetOptions( 'help|?' => \$help, @@ -305,7 +308,7 @@ GetOptions( 'itemscontent=s' => \$itemscontent, 'list-all' => \$listall, 't|triggered' => \$triggered, - 'date' => \$date, + 'date=s' => \$date_input, 'borcat=s' => \@myborcat, 'borcatout=s' => \@myborcatout, 'email=s' => \@emails, @@ -355,13 +358,19 @@ if (@branchcodes) { } } my $date_to_run; -if ($date){ - $date=$dbh->quote($date); - $date_to_run = dt_from_string($date); +my $date; +if ( $date_input ){ + $date = $dbh->quote($date); + eval { + $date_to_run = dt_from_string( $date_input ); + }; + die "$date_input is not a valid date, aborting!" + if $@ or not $date_to_run; + } else { $date="NOW()"; - $date_to_run = DateTime->now( time_zone => C4::Context->tz() ); + $date_to_run = dt_from_string(); } # these are the fields that will be substituted into <> @@ -466,7 +475,8 @@ END_SQL while ( my $overdue_rules = $rqoverduerules->fetchrow_hashref ) { PERIOD: foreach my $i ( 1 .. 3 ) { - $verbose and warn "branch '$branchcode', pass $i\n"; + $verbose and warn "branch '$branchcode', categorycode = $overdue_rules->{categorycode} pass $i\n"; + my $mindays = $overdue_rules->{"delay$i"}; # the notice will be sent after mindays days (grace period) my $maxdays = ( $overdue_rules->{ "delay" . ( $i + 1 ) } @@ -474,6 +484,8 @@ END_SQL : ($MAX) ); # issues being more than maxdays late are managed somewhere else. (borrower probably suspended) + next unless defined $mindays; + if ( !$overdue_rules->{"letter$i"} ) { $verbose and warn "No letter$i code for branch '$branchcode'"; next PERIOD; @@ -571,7 +583,7 @@ END_SQL my $letter = C4::Letters::getletter( 'circulation', $overdue_rules->{"letter$i"}, $branchcode ); unless ($letter) { - $verbose and warn "Message '$overdue_rules->{letter$i}' content not found"; + $verbose and warn qq|Message '$overdue_rules->{"letter$i"}' content not found|; # might as well skip while PERIOD, no other borrowers are going to work. # FIXME : Does this mean a letter must be defined in order to trigger a debar ? @@ -591,9 +603,9 @@ END_SQL ); $verbose and warn "debarring $borr\n"; } -# my @params = ($listall ? ( $borrowernumber , 1 , $MAX ) : ( $borrowernumber, $mindays, $maxdays )); my @params = ($borrowernumber); $verbose and warn "STH2 PARAMS: borrowernumber = $borrowernumber"; + $sth2->execute(@params); my $itemcount = 0; my $titles = ""; @@ -669,7 +681,7 @@ END_SQL } ); unless ($letter) { - $verbose and warn "Message '$overdue_rules->{letter$i}' content not found"; + $verbose and warn qq|Message '$overdue_rules->{"letter$i"}' content not found|; # this transport doesn't have a configured notice, so try another next; } -- 1.9.1