From 2bbcde450b2efab26707c1e560897130bb4334aa Mon Sep 17 00:00:00 2001 From: Alex Arnaud Date: Fri, 15 Sep 2017 12:21:01 +0000 Subject: [PATCH] Bug 15261: Check reserves while renewing an issue Test plan: - Create an issue (i.e from 15/09/2017 to 04/10/2017), - place a reserve on the same item from 05/10/2017 to 30/10/2017), - enable PreventCheckoutOnSameReservePeriod, - Check that you are not able to renew the issue. Rebased (2018-03-15) Rebased (2019-04-18) Rebased-by: Alex Arnaud --- C4/Circulation.pm | 22 ++++++++++++- svc/renew | 2 +- t/db_dependent/Circulation.t | 62 +++++++++++++++++++++++++++++++++++- 3 files changed, 83 insertions(+), 3 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 6afc851da0..e7d89f2d4e 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2661,7 +2661,7 @@ already renewed the loan. $error will contain the reason the renewal can not pro =cut sub CanBookBeRenewed { - my ( $borrowernumber, $itemnumber, $override_limit ) = @_; + my ( $borrowernumber, $itemnumber, $override_limit, $date_due ) = @_; my $dbh = C4::Context->dbh; my $renews = 1; @@ -2738,6 +2738,26 @@ sub CanBookBeRenewed { } } } + + unless ($date_due) { + my $itemtype = $item->effective_itemtype; + my $patron_unblessed = $patron->unblessed; + $date_due = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ? + dt_from_string( $issue->date_due, 'sql' ) : + DateTime->now( time_zone => C4::Context->tz()); + $date_due = CalcDateDue($date_due, $itemtype, _GetCircControlBranch($item, $patron_unblessed), $patron_unblessed, 'is a renewal'); + } + + my $now = dt_from_string(); + my $biblionumber = $item->biblionumber; + my $preventCheckoutOnSameReservePeriod = + C4::Context->preference("PreventCheckoutOnSameReservePeriod"); + my $reserves_on_same_period = + ReservesOnSamePeriod($biblionumber, $itemnumber, $now->ymd, $date_due->ymd); + if ($preventCheckoutOnSameReservePeriod && $reserves_on_same_period) { + $resfound = 1; + } + return ( 0, "on_reserve" ) if $resfound; # '' when no hold was found return ( 1, undef ) if $override_limit; diff --git a/svc/renew b/svc/renew index 29e51d8cf7..f94ef28e9a 100755 --- a/svc/renew +++ b/svc/renew @@ -57,7 +57,7 @@ $data->{borrowernumber} = $borrowernumber; $data->{branchcode} = $branchcode; ( $data->{renew_okay}, $data->{error} ) = - CanBookBeRenewed( $borrowernumber, $itemnumber, $override_limit ); + CanBookBeRenewed( $borrowernumber, $itemnumber, $override_limit, $date_due ); # If we're allowing reserved items to be renewed... if ( $data->{error} && $data->{error} eq 'on_reserve' && C4::Context->preference('AllowRenewalOnHoldOverride')) { diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index b7acf2a617..6c80fb642e 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -18,7 +18,7 @@ use Modern::Perl; use utf8; -use Test::More tests => 45; +use Test::More tests => 47; use Test::MockModule; use Data::Dumper; @@ -382,6 +382,7 @@ subtest "CanBookBeRenewed tests" => sub { my $borrowing_borrowernumber = Koha::Checkouts->find( { itemnumber => $item_1->itemnumber } )->borrowernumber; is ($borrowing_borrowernumber, $renewing_borrowernumber, "Item checked out to $renewing_borrower->{firstname} $renewing_borrower->{surname}"); + t::lib::Mocks::mock_preference('CircControl', 'PatronLibrary'); my ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1); is( $renewokay, 1, 'Can renew, no holds for this title or item'); @@ -1250,6 +1251,65 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { is( $error, 'onsite_checkout', 'A correct error code should be returned by CanBookBeRenewed for on-site checkout' ); } +{ + # Don't allow renewing on reserve period with PreventCheckoutOnSameReservePeriod enabled + t::lib::Mocks::mock_preference( 'PreventCheckoutOnSameReservePeriod', 1 ); + + my $start_issue_dt = DateTime->now(); + $start_issue_dt->subtract( days => 15); + my $due_date = $start_issue_dt->clone; + $due_date->add( days => 17 ); + + my $biblio = $builder->build({ + source => 'Biblio', + }); + my $biblionumber = $biblio->{biblionumber}; + + my $item = $builder->build({ + source => 'Item', + value => { + biblionumber => $biblionumber + } + }); + my $itemnumber = $item->{itemnumber}; + + my $issue = $builder->build({ + source => 'Issue', + value => { + itemnumber => $itemnumber, + biblionumber => $biblionumber, + issuedate => $start_issue_dt->ymd, + date_due => $due_date->ymd, + onsite_checkout => 0 + } + }); + + my $reservedate = $due_date->clone; + $reservedate->add( days => 5 ); + my $expirationdate = $reservedate->clone; + $expirationdate->add( days => 15 ); + $builder->build({ + source => 'Reserve', + value => { + biblionumber => $biblionumber, + itemnumber => $itemnumber, + reservedate => $reservedate->ymd, + expirationdate => $expirationdate->ymd + } + }); + + my $requested_date_due = $due_date->clone; + $requested_date_due->add( days => 4 ); + my ( $renewed, $error ) = CanBookBeRenewed( $issue->{borrowernumber}, $itemnumber, 1, $requested_date_due ); + is( $renewed, 1, 'CanBookBeRenewed should allow to renew if no reserve date overlap' ); + + $requested_date_due->add( days => 2 ); + ( $renewed, $error ) = CanBookBeRenewed( $issue->{borrowernumber}, $itemnumber, 1, $requested_date_due ); + is( $renewed, 0, 'CanBookBeRenewed should not allow to renew if reserve date overlap' ); + + t::lib::Mocks::mock_preference( 'PreventCheckoutOnSameReservePeriod', 0 ); +} + { my $library = $builder->build({ source => 'Branch' }); -- 2.20.1