From 79122f1a8d874195fb948c73d9d3d70a5e01eebb 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. --- C4/Circulation.pm | 24 +++++++++++++++-- svc/renew | 2 +- t/db_dependent/Circulation.t | 63 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 85 insertions(+), 4 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index d5c3f5a..8b3e33e 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2623,12 +2623,12 @@ 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; - my $item = GetItem($itemnumber) or return ( 0, 'no_item' ); + my $item = Koha::Items->find( $itemnumber ) or return ( 0, 'no_item' );; my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return ( 0, 'no_checkout' ); return ( 0, 'onsite_checkout' ) if $issue->onsite_checkout; @@ -2701,6 +2701,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 1aa9a28..2a03e1f 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 34728d3..7d2241d 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 100; +use Test::More tests => 102; use DateTime; @@ -219,6 +219,7 @@ C4::Context->dbh->do("DELETE FROM accountlines"); my ($biblionumber, $biblioitemnumber) = AddBiblio($biblio, ''); + my $barcode = 'R00000342'; my $branch = $library2->{branchcode}; @@ -318,6 +319,7 @@ C4::Context->dbh->do("DELETE FROM accountlines"); my $borrowing_borrowernumber = Koha::Checkouts->find( { itemnumber => $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, $itemnumber, 1); is( $renewokay, 1, 'Can renew, no holds for this title or item'); @@ -1089,6 +1091,65 @@ C4::Context->dbh->do("DELETE FROM accountlines"); } { + # 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' }); my $biblio = MARC::Record->new(); -- 2.7.4