From 68575f8a45a4146d0ed75497ff3b63051989cbcc Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 25 Apr 2016 21:52:27 +0100 Subject: [PATCH] Bug 16344: Add a circ rule to limit the auto renewals given a specific This patch adds a new circulation rule (no_auto_renewal_after_hard_limit) to block/allow auto renewals after a given date. The idea is to stop renewals at a given date. That way the library will have time to send overdues and get the books back before the students do on holiday. Test plan: 0/ Execute the update DB entry 1/ Define a rule with no_auto_renewal_after_hard_limit set to tomorrow 2/ Modify the issues.issuedate, to simulate a checkout in the past: UPDATE issues SET issuedate = "yyyy-mm-dd hh:mm:ss" WHERE itemnumber = YOUR_ITEMNUMBER; with issuedate = 2 days before for instance 3/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl) Confirm that the issue has been renewed 4/ Modify the no_auto_renewal_after_hard_limit and set it to yesterday 5/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl) Confirm that the issue has not been renewed Signed-off-by: Jonathan Field Signed-off-by: Janet McGowan --- C4/Circulation.pm | 59 ++++++++++++++-------- admin/smart-rules.pl | 9 ++++ installer/data/mysql/atomicupdate/bug_16344.sql | 1 + installer/data/mysql/kohastructure.sql | 1 + .../prog/en/modules/admin/smart-rules.tt | 6 +++ t/db_dependent/Circulation.t | 44 +++++++++++++--- 6 files changed, 93 insertions(+), 27 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_16344.sql diff --git a/C4/Circulation.pm b/C4/Circulation.pm index af5acce..074706f 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2721,19 +2721,26 @@ sub CanBookBeRenewed { return ( 0, 'overdue'); } - if ( $itemissue->{auto_renew} - and defined $issuing_rule->no_auto_renewal_after + if ( $itemissue->{auto_renew} ) { + if ( defined $issuing_rule->no_auto_renewal_after and $issuing_rule->no_auto_renewal_after ne "" ) { - - # Get issue_date and add no_auto_renewal_after - # If this is greater than today, it's too late for renewal. - my $maximum_renewal_date = dt_from_string($itemissue->{issuedate}); - $maximum_renewal_date->add( - $issuing_rule->lengthunit => $issuing_rule->no_auto_renewal_after - ); - my $now = dt_from_string; - if ( $now >= $maximum_renewal_date ) { - return ( 0, "auto_too_late" ); + # Get issue_date and add no_auto_renewal_after + # If this is greater than today, it's too late for renewal. + my $maximum_renewal_date = dt_from_string($itemissue->{issuedate}); + $maximum_renewal_date->add( + $issuing_rule->lengthunit => $issuing_rule->no_auto_renewal_after + ); + my $now = dt_from_string; + if ( $now >= $maximum_renewal_date ) { + return ( 0, "auto_too_late" ); + } + } + if ( defined $issuing_rule->no_auto_renewal_after_hard_limit + and $issuing_rule->no_auto_renewal_after_hard_limit ne "" ) { + # If no_auto_renewal_after_hard_limit is >= today, it's also too late for renewal + if ( dt_from_string >= dt_from_string( $issuing_rule->no_auto_renewal_after_hard_limit ) ) { + return ( 0, "auto_too_late" ); + } } } @@ -3035,8 +3042,8 @@ has the item on loan. C<$itemnumber> is the number of the item to renew. C<$GetLatestAutoRenewDate> returns the DateTime of the latest possible -auto renew date, based on the value "No auto renewal after" of the applicable -issuing rule. +auto renew date, based on the value "No auto renewal after" and the "No auto +renewal after (hard limit) of the applicable issuing rule. Returns undef if there is no date specify in the circ rules or if the patron, loan, or item cannot be found. @@ -3063,14 +3070,24 @@ sub GetLatestAutoRenewDate { ); return unless $issuing_rule; - return if not $issuing_rule->no_auto_renewal_after - or $issuing_rule->no_auto_renewal_after eq ''; - - my $maximum_renewal_date = dt_from_string($itemissue->{issuedate}); - $maximum_renewal_date->add( - $issuing_rule->lengthunit => $issuing_rule->no_auto_renewal_after - ); + return + if ( not $issuing_rule->no_auto_renewal_after + or $issuing_rule->no_auto_renewal_after eq '' ) + and ( not $issuing_rule->no_auto_renewal_after_hard_limit + or $issuing_rule->no_auto_renewal_after_hard_limit eq '' ); + + my $maximum_renewal_date; + if ( $issuing_rule->no_auto_renewal_after ) { + $maximum_renewal_date = dt_from_string($itemissue->{issuedate}); + $maximum_renewal_date->add( + $issuing_rule->lengthunit => $issuing_rule->no_auto_renewal_after + ); + } + if ( $issuing_rule->no_auto_renewal_after_hard_limit ) { + my $dt = dt_from_string( $issuing_rule->no_auto_renewal_after_hard_limit ); + $maximum_renewal_date = $dt if not $maximum_renewal_date or $maximum_renewal_date > $dt; + } return $maximum_renewal_date; } diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index ba8b5bf..8346f63 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -138,6 +138,9 @@ elsif ($op eq 'add') { my $auto_renew = $input->param('auto_renew') eq 'yes' ? 1 : 0; my $no_auto_renewal_after = $input->param('no_auto_renewal_after'); $no_auto_renewal_after = undef if $no_auto_renewal_after =~ /^\s*$/; + my $no_auto_renewal_after_hard_limit = $input->param('no_auto_renewal_after_hard_limit') || undef; + $no_auto_renewal_after_hard_limit = eval { dt_from_string( $input->param('no_auto_renewal_after_hard_limit') ) } if ( $no_auto_renewal_after_hard_limit ); + $no_auto_renewal_after_hard_limit = output_pref( { dt => $no_auto_renewal_after_hard_limit, dateonly => 1, dateformat => 'iso' } ) if ( $no_auto_renewal_after_hard_limit ); my $reservesallowed = $input->param('reservesallowed'); my $holds_per_record = $input->param('holds_per_record'); my $onshelfholds = $input->param('onshelfholds') || 0; @@ -176,6 +179,7 @@ elsif ($op eq 'add') { norenewalbefore => $norenewalbefore, auto_renew => $auto_renew, no_auto_renewal_after => $no_auto_renewal_after, + no_auto_renewal_after_hard_limit => $no_auto_renewal_after_hard_limit, reservesallowed => $reservesallowed, holds_per_record => $holds_per_record, issuelength => $issuelength, @@ -504,6 +508,11 @@ while (my $row = $sth2->fetchrow_hashref) { } else { $row->{'hardduedate'} = 0; } + if ($row->{no_auto_renewal_after_hard_limit}) { + my $dt = eval { dt_from_string( $row->{no_auto_renewal_after_hard_limit} ) }; + $row->{no_auto_renewal_after_hard_limit} = eval { output_pref( { dt => $dt, dateonly => 1 } ) } if $dt; + } + push @row_loop, $row; } diff --git a/installer/data/mysql/atomicupdate/bug_16344.sql b/installer/data/mysql/atomicupdate/bug_16344.sql new file mode 100644 index 0000000..5be20e8 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_16344.sql @@ -0,0 +1 @@ +ALTER TABLE issuingrules ADD COLUMN no_auto_renewal_after_hard_limit DATE DEFAULT NULL AFTER no_auto_renewal_after; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 91e6119..79d229a 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -869,6 +869,7 @@ CREATE TABLE `issuingrules` ( -- circulation and fine rules `norenewalbefore` int(4) default NULL, -- no renewal allowed until X days or hours before due date. `auto_renew` BOOLEAN default FALSE, -- automatic renewal `no_auto_renewal_after` int(4) default NULL, -- no auto renewal allowed after X days or hours after the issue date + `no_auto_renewal_after_hard_limit` date default NULL, -- no auto renewal allowed after a given date `reservesallowed` smallint(6) NOT NULL default "0", -- how many holds are allowed `holds_per_record` SMALLINT(6) NOT NULL DEFAULT 1, -- How many holds a patron can have on a given bib `branchcode` varchar(10) NOT NULL default '', -- the branch this rule is for (branches.branchcode) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt index 746d0c8..ec6c45a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -186,6 +186,7 @@ $(document).ready(function() { No renewal before Automatic renewal No automatic renewal after + No automatic renewal after (hard limit) Holds allowed (count) Holds per record (count) On shelf holds allowed @@ -267,6 +268,7 @@ $(document).ready(function() { [% END %] [% rule.no_auto_renewal_after %] + [% rule.no_auto_renewal_after_hard_limit %] [% rule.reservesallowed %] [% rule.holds_per_record %] @@ -354,6 +356,10 @@ $(document).ready(function() { + + +
[% INCLUDE 'date-format.inc' %]
+ diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 352abf8..52aa087 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -566,7 +566,7 @@ C4::Context->dbh->do("DELETE FROM accountlines"); ); subtest "too_late_renewal / no_auto_renewal_after" => sub { - plan tests => 8; + plan tests => 14; my $item_to_auto_renew = $builder->build( { source => 'Item', value => { @@ -604,10 +604,28 @@ C4::Context->dbh->do("DELETE FROM accountlines"); CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Cannot renew, renew is automatic' ); + + $dbh->do('UPDATE issuingrules SET norenewalbefore = 7, no_auto_renewal_after = NULL, no_auto_renewal_after_hard_limit = ?', undef, dt_from_string->add( days => -1 ) ); + ( $renewokay, $error ) = + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + is( $renewokay, 0, 'Do not renew, renewal is automatic' ); + is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' ); + + $dbh->do('UPDATE issuingrules SET norenewalbefore = 7, no_auto_renewal_after = 15, no_auto_renewal_after_hard_limit = ?', undef, dt_from_string->add( days => -1 ) ); + ( $renewokay, $error ) = + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + is( $renewokay, 0, 'Do not renew, renewal is automatic' ); + is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' ); + + $dbh->do('UPDATE issuingrules SET norenewalbefore = 10, no_auto_renewal_after = NULL, no_auto_renewal_after_hard_limit = ?', undef, dt_from_string->add( days => 1 ) ); + ( $renewokay, $error ) = + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + is( $renewokay, 0, 'Do not renew, renewal is automatic' ); + is( $error, 'auto_renew', 'Cannot renew, renew is automatic' ); }; subtest "GetLatestAutoRenewDate" => sub { - plan tests => 3; + plan tests => 5; my $item_to_auto_renew = $builder->build( { source => 'Item', value => { @@ -621,23 +639,37 @@ C4::Context->dbh->do("DELETE FROM accountlines"); my $ten_days_before = dt_from_string->add( days => -10 ); my $ten_days_ahead = dt_from_string->add( days => 10 ); AddIssue( $renewing_borrower, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); - $dbh->do('UPDATE issuingrules SET norenewalbefore = 7, no_auto_renewal_after = ""'); + $dbh->do('UPDATE issuingrules SET norenewalbefore = 7, no_auto_renewal_after = "", no_auto_renewal_after_hard_limit = NULL'); my $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); - is( $latest_auto_renew_date, undef, 'GetLatestAutoRenewDate should return undef if no_auto_renewal_after is not defined' ); + is( $latest_auto_renew_date, undef, 'GetLatestAutoRenewDate should return undef if no_auto_renewal_after or no_auto_renewal_after_hard_limit are not defined' ); my $five_days_before = dt_from_string->add( days => -5 ); - $dbh->do('UPDATE issuingrules SET norenewalbefore = 10, no_auto_renewal_after = 5'); + $dbh->do('UPDATE issuingrules SET norenewalbefore = 10, no_auto_renewal_after = 5, no_auto_renewal_after_hard_limit = NULL'); $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); is( $latest_auto_renew_date->truncate( to => 'minute' ), $five_days_before->truncate( to => 'minute' ), 'GetLatestAutoRenewDate should return -5 days if no_auto_renewal_after = 5 and date_due is 10 days before' ); my $five_days_ahead = dt_from_string->add( days => 5 ); - $dbh->do('UPDATE issuingrules SET norenewalbefore = 10, no_auto_renewal_after = 15'); + $dbh->do('UPDATE issuingrules SET norenewalbefore = 10, no_auto_renewal_after = 15, no_auto_renewal_after_hard_limit = NULL'); $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); is( $latest_auto_renew_date->truncate( to => 'minute' ), $five_days_ahead->truncate( to => 'minute' ), 'GetLatestAutoRenewDate should return +5 days if no_auto_renewal_after = 15 and date_due is 10 days before' ); + my $two_days_ahead = dt_from_string->add( days => 2 ); + $dbh->do('UPDATE issuingrules SET norenewalbefore = 10, no_auto_renewal_after = "", no_auto_renewal_after_hard_limit = ?', undef, dt_from_string->add( days => 2 ) ); + $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + is( $latest_auto_renew_date->truncate( to => 'day' ), + $two_days_ahead->truncate( to => 'day' ), + 'GetLatestAutoRenewDate should return +2 days if no_auto_renewal_after_hard_limit is defined and not no_auto_renewal_after' + ); + $dbh->do('UPDATE issuingrules SET norenewalbefore = 10, no_auto_renewal_after = 15, no_auto_renewal_after_hard_limit = ?', undef, dt_from_string->add( days => 2 ) ); + $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + is( $latest_auto_renew_date->truncate( to => 'day' ), + $two_days_ahead->truncate( to => 'day' ), + 'GetLatestAutoRenewDate should return +2 days if no_auto_renewal_after_hard_limit is < no_auto_renewal_after' + ); + }; # Too many renewals -- 2.1.4