From 54b2c45bc5caeea9065e1f9cc59dcdd36a18717b 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 68b95d3021..20bb40de0c 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2622,7 +2622,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; @@ -2699,6 +2699,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 1aa9a28944..2a03e1f847 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 ( $data->{renew_okay} ) { $date_due = AddRenewal( $borrowernumber, $itemnumber, $branchcode, $date_due ); diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index f3e5b1b94a..eaa5ee3d7a 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 => 127; +use Test::More tests => 129; use Test::MockModule; use Data::Dumper; @@ -327,6 +327,7 @@ my ( $reused_itemnumber_1, $reused_itemnumber_2 ); 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'); @@ -1192,6 +1193,65 @@ my ( $reused_itemnumber_1, $reused_itemnumber_2 ); 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