Bugzilla – Attachment 22304 Details for
Bug 11120
The date option for the overdue_notices cronjob is broken
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 11120: FIX the --date option for overdue_notices cronjob
Bug-11120-FIX-the---date-option-for-overduenotices.patch (text/plain), 10.66 KB, created by
Jonathan Druart
on 2013-10-23 08:40:39 UTC
(
hide
)
Description:
Bug 11120: FIX the --date option for overdue_notices cronjob
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2013-10-23 08:40:39 UTC
Size:
10.66 KB
patch
obsolete
>From 75622eb756d612b1abdbd3d23b78759f9e5aa29c Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >Date: Wed, 23 Oct 2013 10:13:50 +0200 >Subject: [PATCH] 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. >--- > misc/cronjobs/overdue_notices.pl | 79 ++++++++++++++++++++++++++++---------- > 1 file changed, 58 insertions(+), 21 deletions(-) > >diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl >index c1c4c19..37bbb0c 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); > use C4::Budgets qw(GetCurrency); >+use Koha::DateUtils; > > =head1 NAME > >@@ -276,7 +279,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, >@@ -290,7 +293,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, >@@ -340,11 +343,16 @@ if (@branchcodes) { > } > } > >-if ($date){ >- $date=$dbh->quote($date); >+if ( $date_input ){ >+ eval { >+ $today = dt_from_string( $date_input ); >+ }; >+ die "$date_input is not a valid date, aborting!" >+ if $@ or not $today; >+ > } > else { >- $date="NOW()"; >+ $today = dt_from_string; > } > > # these are the fields that will be substituted into <<item.content>> >@@ -377,7 +385,6 @@ if ( defined $htmlfilename ) { > if ( $htmlfilename eq '' ) { > $html_fh = *STDOUT; > } else { >- my $today = DateTime->now(time_zone => C4::Context->tz ); > open $html_fh, ">",File::Spec->catdir ($htmlfilename,"notices-".$today->ymd().".html"); > } > >@@ -403,13 +410,13 @@ foreach my $branchcode (@branches) { > $verbose and warn sprintf "branchcode : '%s' using %s\n", $branchcode, $admin_email_address; > > my $sth2 = $dbh->prepare( <<"END_SQL" ); >-SELECT biblio.*, items.*, issues.*, biblioitems.itemtype, TO_DAYS($date)-TO_DAYS(date_due) AS days_overdue >+SELECT biblio.*, items.*, issues.*, biblioitems.itemtype > FROM issues,items,biblio, biblioitems > WHERE items.itemnumber=issues.itemnumber > AND biblio.biblionumber = items.biblionumber > AND biblio.biblionumber = biblioitems.biblionumber > AND issues.borrowernumber = ? >- AND TO_DAYS($date)-TO_DAYS(date_due) BETWEEN ? and ? >+ AND CAST(date_due AS date) BETWEEN ? and ? > END_SQL > > my $query = "SELECT * FROM overduerules WHERE delay1 IS NOT NULL AND branchcode = ? "; >@@ -433,7 +440,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 ) } >@@ -441,6 +449,22 @@ END_SQL > : ($MAX) > ); # issues being more than maxdays late are managed somewhere else. (borrower probably suspended) > >+ next unless defined $mindays; >+ >+ my $range_end = output_pref({ >+ dt => $today->clone->subtract_duration( DateTime::Duration->new( days => $mindays ) ), >+ dateformat => 'sql', >+ dateonly => 1, >+ }); >+ my $range_begin = $triggered >+ ? $range_end >+ : output_pref({ >+ dt => $today->clone->subtract_duration( DateTime::Duration->new( days => $maxdays ) ), >+ dateformat => 'sql', >+ date_only => 1, >+ }); >+ >+ > if ( !$overdue_rules->{"letter$i"} ) { > $verbose and warn "No letter$i code for branch '$branchcode'"; > next PERIOD; >@@ -468,18 +492,14 @@ END_SQL > push @borrower_parameters, $overdue_rules->{categorycode}; > } > $borrower_sql .= ' AND categories.overduenoticerequired=1 '; >- if($triggered) { >- $borrower_sql .= " AND TO_DAYS($date)-TO_DAYS(date_due) = ?"; >- push @borrower_parameters, $mindays; >- } else { >- $borrower_sql .= " AND TO_DAYS($date)-TO_DAYS(date_due) BETWEEN ? and ? " ; >- push @borrower_parameters, $mindays, $maxdays; >- } >+ >+ $borrower_sql .= " AND CAST(date_due AS date) BETWEEN ? and ? " ; >+ push @borrower_parameters, $range_begin, $range_end; > > # $sth gets borrower info iff at least one overdue item has triggered the overdue action. > my $sth = $dbh->prepare($borrower_sql); > $sth->execute(@borrower_parameters); >- $verbose and warn $borrower_sql . "\n $branchcode | " . $overdue_rules->{'categorycode'} . "\n ($mindays, $maxdays)\nreturns " . $sth->rows . " rows"; >+ $verbose and warn $borrower_sql . "\n $branchcode | " . $overdue_rules->{'categorycode'} . "\n ($range_begin, $range_end)\nreturns " . $sth->rows . " rows"; > > while ( my $data = $sth->fetchrow_hashref ) { > my $borrowernumber = $data->{'borrowernumber'}; >@@ -507,7 +527,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 ? >@@ -520,8 +540,25 @@ END_SQL > C4::Members::DebarMember($borrowernumber, '9999-12-31'); > $verbose and warn "debarring $borr\n"; > } >- my @params = ($listall ? ( $borrowernumber , 1 , $MAX ) : ( $borrowernumber, $mindays, $maxdays )); >- $verbose and warn "STH2 PARAMS: borrowernumber = $borrowernumber, mindays = $mindays, maxdays = $maxdays"; >+ >+ my @params = ( $listall >+ ? ( $borrowernumber , output_pref({ >+ dt => $today->clone->subtract_duration( days => 1 ), >+ dateformat => 'sql', >+ date_only => 1 >+ }), >+ output_pref({ >+ dt => $today->clone->subtract_duration( days => $maxdays ), >+ dateformat => 'sql', >+ date_only => 1 >+ }) >+ ) >+ : ( $borrowernumber, $range_begin, $range_end ) >+ ); >+ $verbose and warn $listall >+ ? "STH2 PARAMS: borrowernumber = $borrowernumber, from = NOW - 1, to = NOW - $maxdays, maxdays = $maxdays" >+ : "STH2 PARAMS: borrowernumber = $borrowernumber, from = $range_begin, to = $range_end, maxdays = $maxdays"; >+ > $sth2->execute(@params); > my $itemcount = 0; > my $titles = ""; >@@ -555,7 +592,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|; > > # 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 ? >-- >1.7.10.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 11120
:
22304
|
29465
|
33929
|
34952
|
34953
|
34962
|
34963
|
34979
|
34980
|
34981
|
36282
|
36283
|
36284