Bugzilla – Attachment 29465 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.74 KB, created by
Jonathan Druart
on 2014-07-03 08:16:56 UTC
(
hide
)
Description:
Bug 11120: FIX the --date option for overdue_notices cronjob
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2014-07-03 08:16:56 UTC
Size:
10.74 KB
patch
obsolete
>From 9e94c7b3b84b7f5255abce1fcf2b39b89b260c5b 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 | 80 +++++++++++++++++++++++++++++----------- > 1 file changed, 59 insertions(+), 21 deletions(-) > >diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl >index 2a9f8ae..3e651fe 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; >@@ -289,7 +292,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, >@@ -304,7 +307,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, >@@ -354,11 +357,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>> >@@ -391,7 +399,6 @@ if ( defined $htmlfilename ) { > if ( $htmlfilename eq '' ) { > $fh = *STDOUT; > } else { >- my $today = DateTime->now(time_zone => C4::Context->tz ); > open $fh, ">",File::Spec->catdir ($htmlfilename,"notices-".$today->ymd().".html"); > } > >@@ -425,13 +432,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 = ? "; >@@ -454,7 +461,9 @@ END_SQL > # my $outfile = 'overdues_' . ( $mybranch || $branchcode || 'default' ); > 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 ) } >@@ -462,6 +471,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; >@@ -489,18 +514,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'}; >@@ -528,7 +549,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 ? >@@ -548,8 +569,25 @@ END_SQL > ); > $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 = ""; >@@ -592,7 +630,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; > } >-- >2.0.0.rc2
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