From a77c1034f16b1e94edb374b80c46ed7ac42eeff8 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 --- C4/Circulation.pm | 55 ++++++++++++++-------- 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, 90 insertions(+), 26 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_16344.sql diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 80169da..f5aeef1 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2851,19 +2851,26 @@ sub CanBookBeRenewed { return ( 0, 'overdue'); } - if ( $itemissue->{auto_renew} - and defined $issuingrule->{no_auto_renewal_after} + if ( $itemissue->{auto_renew} ) { + if ( defined $issuingrule->{no_auto_renewal_after} and $issuingrule->{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( - $issuingrule->{lengthunit} => $issuingrule->{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( + $issuingrule->{lengthunit} => $issuingrule->{no_auto_renewal_after} + ); + my $now = dt_from_string; + if ( $now >= $maximum_renewal_date ) { + return ( 0, "auto_too_late" ); + } + } + if ( defined $issuingrule->{no_auto_renewal_after_hard_limit} + and $issuingrule->{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( $issuingrule->{no_auto_renewal_after_hard_limit} ) ) { + return ( 0, "auto_too_late" ); + } } } @@ -3151,8 +3158,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. @@ -3176,14 +3183,22 @@ sub GetLatestAutoRenewDate { my $now = dt_from_string; - return if not $issuingrule->{no_auto_renewal_after} - or $issuingrule->{no_auto_renewal_after} eq ''; + return if ( not $issuingrule->{no_auto_renewal_after} or $issuingrule->{no_auto_renewal_after} eq '' ) + and ( not $issuingrule->{no_auto_renewal_after_hard_limit} or $issuingrule->{no_auto_renewal_after_hard_limit} eq '' ); - my $maximum_renewal_date = dt_from_string($itemissue->{issuedate}); - $maximum_renewal_date->add( - $issuingrule->{lengthunit} => $issuingrule->{no_auto_renewal_after} - ); + my $maximum_renewal_date; + if ( $issuingrule->{no_auto_renewal_after} ) { + $maximum_renewal_date = dt_from_string($itemissue->{issuedate}); + $maximum_renewal_date->add( + $issuingrule->{lengthunit} => $issuingrule->{no_auto_renewal_after} + ); + } + + if ( $issuingrule->{no_auto_renewal_after_hard_limit} ) { + my $dt = dt_from_string( $issuingrule->{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 8d62c2c..9c60037 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; @@ -175,6 +178,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, @@ -502,6 +506,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 225afd9..c7ef23d 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -870,6 +870,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 cb5db63..1ded4e3 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 @@ -266,6 +267,7 @@ $(document).ready(function() { [% END %] [% rule.no_auto_renewal_after %] + [% rule.no_auto_renewal_after_hard_limit %] [% rule.reservesallowed %] [% rule.holds_per_record %] @@ -342,6 +344,10 @@ $(document).ready(function() { + + +
[% INCLUDE 'date-format.inc' %]
+ diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 96cfc28..a8eebd1 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -551,7 +551,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 => { @@ -589,10 +589,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 => { @@ -606,23 +624,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